feat: add smart and partial delegation #3

0xRaccoon · 2023-12-18T16:39:37Z

Signed-off-by: 0xRaccoon <[email protected]>

0xParticle · 2023-12-18T17:54:07Z

solidity/contracts/governance/utils/WonderVotes.sol


-  mapping(address account => address) private _delegatee;
+  mapping(address account => mapping(uint8 proposalType => Delegate[])) private _delegatees;


have you considered to use a single mapping with the hash(account, proposalType)? I guess it would be cheaper

Yes, I considered it. We could discuss it. The main tradeoff that I see with the hash approach is that we would have to perform a hashing operation when the user votes and when delegates (in this case 1, hashing operation per account-proposalType) combination.

@skeletor-spaceman any thoughts about this one?

hashing is done either way on the mapping, so it does not really optimizes anything, nested mappings are easier and cleaner to read.

0xParticle · 2023-12-18T17:54:19Z

solidity/contracts/governance/utils/WonderVotes.sol


-  mapping(address delegatee => Checkpoints.Trace208) private _delegateCheckpoints;
+  mapping(address delegatee => mapping(uint8 proposalType => Checkpoints.Trace208)) private _delegateCheckpoints;


0xParticle · 2023-12-18T17:56:08Z

solidity/contracts/governance/utils/WonderVotes.sol

+      if (delegatees[i].weight == 0) revert ZeroWeight();
+      _weightSum += delegatees[i].weight;
+    }
+    if (_weightSum != _totalWeight()) revert InvalidWeightSum(_weightSum);


an alternative approach (but don't think its worth) is normalizing, i.e. considering the proportion and translating to the real weights

that would even allow users to input percentages for example instead of precise uints

mmm not sure if I get this one, could you provide an example?. If I translate to real weight wouldn't that one be a float?.

In this approach, the protocol implements the totalWeight() which will represent the 100% of weight that is used a control that when delegating the weightSum of the delegatees is always equal to totalWeight.

Eg. Let's suppose that a user delegates his votes (let's abstract the token decimals since it's transparent for us, and also the proposalTypes since we know that it will apply to all of them):

totalWeight = 255; (defined by the protocol and applies to all delegations)
tokens held by user = 100;

The user delegates to 3 different delegates, knowing that he has to distribute a total 255 in weights units

delegate1_weight = 95; delegate2_weight = 50; delegate3_weight = 110;

The formula is:
voting_power = delegator_tokens_held * weight / totalWeight;

The vote power distribution will be:

delegate1_votes = 100 * 95 / 255 => ≈ 37.25; delegate2_votes = 100 * 50 / 255 => ≈ 19.60; delegate3_votes = 100 * 110 / 255 => ≈ 43.14;

this block of code only controls that the weightSum is 100%, _moveDelegateVotes is the one that calculates the votes according to each weight

Really appreciate the details! What I meant was maybe not reverting if the sum of the weights doesnt add up to totalWeight(), and just finding out the proportion.

Example: suppose totalWeight=255, then a user can define

delegate1_weight = 20; //20% delegate2_weight = 50; // 50% delegate3_weight = 30; //30%

and then you can compute uint256 sumedWeights = delegate1_weight + delegate2_weight + delegate3_weight, and consider the proportion as (delegate1_weight * totalWeight/sumedWeights).
Weights can be anything here, doesnt even need to add up to 100. Just compute the proportion.

I'm not sure if this is better, I think its a little bit more user friendly, but I might be mistaken

I considered this approach using relative weights; it's more flexible. The problem here might be that, in some conditions, it could overflow; let's suppose that a user delegates to two delegates with a considerable weight number but with a low balance. Then, if the user's balance significantly increments, for example, it could overflow when transferring the tokens and give the protocol less control over that.

ok, so end of the day, using totalWeight is a way of ensuring amounts are not excesive. Another way to avoid this while keeping relative weights is by using a maxAmount, like forcing the sum up to 100 (see example above). Again, I'm not sure which one is better UX wise, but I lean to think percentages over amounts is more clear.

yes we could do this control, we would still have the downside that when transferring tokens for example we will always have to sum up all the weights of the arrays instead of using a straight calculation which will introduce more overhead

0xParticle · 2023-12-18T18:01:05Z

I'm a bit confused by the use of totalWeight. Where is it defined? Does it use percentages that sum up to 1 (or 100) or is it an amount for each delegatee?
The rest looks clean to me

Signed-off-by: 0xRaccoon <[email protected]>

0xRaccoon · 2023-12-18T19:27:44Z

I'm a bit confused by the use of totalWeight. Where is it defined? Does it use percentages that sum up to 1 (or 100) or is it an amount for each delegatee? The rest looks clean to me

Since Votes is an abstract contract then we can define some methods that will need to be overridden by the implementation token and that can be up to each protocol. One of them is totalWeight() this one is used to calculate the voting power distribution between delegates and will represent how granular is the weight precision, In the current implementation we used a uint8 but we could even define a more precise type like uint256 if we think that it will be worth, It will be responsibility of the protocols to check for the overflows if they decide to use a big totalWeight.
If the user defines a totalWeight = 255 this means that 100% weight = 255 and will be always used as a divisor to calculate the voting power

Signed-off-by: 0xRaccoon <[email protected]>

skeletor-spaceman · 2023-12-19T14:09:19Z

solidity/contracts/governance/utils/WonderVotes.sol

-    address oldDelegate = delegates(account);
-    _delegatee[account] = delegatee;
+  function _delegate(address account, uint8 proposalType, Delegate[] memory delegatees) internal virtual {
+    if (delegatees.length > _maxDelegates()) revert DelegatesMaxNumberExceeded(delegatees.length);


what is _maxDelegates used for? total maxDelegates of a user? or per operation?

the max number of delegates that a user can delegate for a proposalType. This applies to all proposals and it's defined by the protocol

got it, so on _delegate you need to always supply the entire list. you cannot add or modify only 1 (or a subset) delegatees. right? that makes sense

skeletor-spaceman · 2023-12-19T14:15:12Z

solidity/contracts/governance/utils/WonderVotes.sol

+      if (delegatees[i].weight == 0) revert ZeroWeight();
+      _weightSum += delegatees[i].weight;
+    }
+    if (_weightSum != _totalWeight()) revert InvalidWeightSum(_weightSum);


why does it need to be equal? can it not just be lower or equal than total? i.e. you don't want to assign all of your weight.

also, what happens when you transfer tokens out?
who out of these delegatees will have a reduced weight? all of them? that's a bit expensive gas-wise
and on tokens in? who gets the increase in weight? all of them or none?
maybe we should consider an ordered list of delegatees, where votes are taken from the bottom-up and assigned to the top one. since looping on proposalTypes + delegatees is going to be a gas nightmare

This is done for optimization every time you decide to distribute your voting power for proposal you will have to define the entire distribution. If we don't delegate the 100% of the weight, then we will have to allow the user to add more delegations to the same proposals, controlling the percentages that we already assigned which will introduce overhead. Another way if you mean this would be if delegating a percentage of weight that is not the 100% means that the user always keeps the rest of the voting power. And a new delegation will override the previous one for a proposalType that would make sense. But kinda could be the same that the current approach if the user delegates himself in the array with the weight desired to keep.

when transferring tokens _transferVotingUnits is called, adding or substracting voting power to the corresponding addresses.
All the delegates with the from account for all the proposalTypes votes will be reduced and all the delegates with the to will be incremented. (_transferVotingUnits and _moveDelegateVotes) functions.

Could you expand on the ordered list of delegates solution?. Since all the delegates from the sender and all the delegates of the destination for all proposalTypes will be affected I'm not sure how this will help with the gas efficiency

Could you expand on the ordered list of delegates solution?. Since all the delegates from the sender and all the delegates of the destination for all proposalTypes will be affected I'm not sure how this will help with the gas efficiency

yes, i'm trying to figure out a way to avoid having to loop over all proposalTypes * all delegatees. but it might not be a desirable UX.

Signed-off-by: 0xRaccoon <[email protected]>

…or-poc into feat/add-smart-and-partial-delegation Signed-off-by: 0xRaccoon <[email protected]>

feat: add smart and partial delegation

a7719cd

Signed-off-by: 0xRaccoon <[email protected]>

0xRaccoon requested review from eenti, skeletor-spaceman, 0xParticle and 0xr3x December 18, 2023 16:39

0xParticle reviewed Dec 18, 2023

View reviewed changes

fix: add proposalType validity check

d7d9aa0

Signed-off-by: 0xRaccoon <[email protected]>

fix: use uint256 type for weights

a041f0c

Signed-off-by: 0xRaccoon <[email protected]>

0xRaccoon mentioned this pull request Dec 19, 2023

feat: implement wonder votes #4

Merged

skeletor-spaceman reviewed Dec 19, 2023

View reviewed changes

0xParticle previously approved these changes Dec 19, 2023

View reviewed changes

fix: rename totalWeight to weightNormalizer

c58841a

Signed-off-by: 0xRaccoon <[email protected]>

0xRaccoon dismissed 0xParticle’s stale review via c58841a December 19, 2023 15:52

0xRaccoon requested review from skeletor-spaceman and 0xParticle December 19, 2023 20:52

Merge branch 'dev' of github-defi:defi-wonderland/wonderland-governoo…

80af104

…or-poc into feat/add-smart-and-partial-delegation Signed-off-by: 0xRaccoon <[email protected]>

0xRaccoon requested a review from gas1cent December 20, 2023 13:46

gas1cent approved these changes Dec 20, 2023

View reviewed changes

0xParticle approved these changes Dec 20, 2023

View reviewed changes

0xRaccoon merged commit c4a138a into dev Dec 20, 2023
6 checks passed

0xRaccoon deleted the feat/add-smart-and-partial-delegation branch December 20, 2023 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add smart and partial delegation #3

feat: add smart and partial delegation #3

0xRaccoon commented Dec 18, 2023

0xParticle Dec 18, 2023

0xRaccoon Dec 18, 2023

0xRaccoon Dec 18, 2023

skeletor-spaceman Dec 19, 2023

0xParticle Dec 18, 2023

0xParticle Dec 18, 2023

0xParticle Dec 18, 2023

0xRaccoon Dec 18, 2023 •

edited

Loading

0xRaccoon Dec 18, 2023

0xParticle Dec 19, 2023

0xRaccoon Dec 19, 2023

0xParticle Dec 19, 2023

0xRaccoon Dec 19, 2023

0xParticle commented Dec 18, 2023 •

edited

Loading

0xRaccoon commented Dec 18, 2023 •

edited

Loading

skeletor-spaceman Dec 19, 2023

0xRaccoon Dec 19, 2023 •

edited

Loading

skeletor-spaceman Dec 19, 2023

0xRaccoon Dec 19, 2023

skeletor-spaceman Dec 19, 2023

0xRaccoon Dec 19, 2023

skeletor-spaceman Dec 19, 2023 •

edited

Loading


		mapping(address account => address) private _delegatee;
		mapping(address account => mapping(uint8 proposalType => Delegate[])) private _delegatees;


		mapping(address delegatee => Checkpoints.Trace208) private _delegateCheckpoints;
		mapping(address delegatee => mapping(uint8 proposalType => Checkpoints.Trace208)) private _delegateCheckpoints;

feat: add smart and partial delegation #3

feat: add smart and partial delegation #3

Conversation

0xRaccoon commented Dec 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xRaccoon Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xParticle commented Dec 18, 2023 • edited Loading

0xRaccoon commented Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

0xRaccoon Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skeletor-spaceman Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

0xRaccoon Dec 18, 2023 •

edited

Loading

0xParticle commented Dec 18, 2023 •

edited

Loading

0xRaccoon commented Dec 18, 2023 •

edited

Loading

0xRaccoon Dec 19, 2023 •

edited

Loading

skeletor-spaceman Dec 19, 2023 •

edited

Loading