PROTO: Read voting power from chain #21
Conversation
| // TODO: Should we limit the length of the chain? | ||
|
|
||
| // Continue building the chain recursively | ||
| for (uint i = 0; i < backwardSubdelegationKeys[current].length; i++) { |
There was a problem hiding this comment.
backwardSubdelegationKeys array can be infinitely and cheaply expanded for by subdelegating from multiple addresses.
loops over dynamic unbounded arrays are the reason why this approach susceptible to DOS attack, as this function can easily be made to exceed block size limit and thus voter might be unable to vote due to someone else's actions
you could try to get around it by processing n values each time, but that then requires a much more complex logic and multiple transactions which kind of defeat the purpose.
| ) { | ||
| k = i; | ||
| // Do the same for forwardSubdelegationKeys | ||
| if (forwardSubdelegationKeys[to].length == 0) { |
There was a problem hiding this comment.
It looks like potential DoS issue in case forwardSubdelegationKeys will be large enough. Similar to comment from @jjranalli regarding the backwardSubdelegationKeys.
Possible solutions:
- Limit max number of these delegation keys?
- Use mapping (?) - need to review if this possible.
There was a problem hiding this comment.
P.S coud you please check whether EnumerableSet help us for searching purposes? It has complexity O(1) which would help for our case. We can use "contains" method
| return 0; | ||
| } | ||
|
|
||
| uint265 proxyBalance = IVotes(OP_TOKEN).getPastVotes(proxyAddress(from), blockNumber); |
| } | ||
|
|
||
| // Draining proxies starting from the closest to the voter | ||
| function _drainProxies(address current, uint265 remainingBalance) internal { |
| if (k == 0) { | ||
| if ( | ||
| subdelegationRules.allowance | ||
| < (subdelegationRules.allowanceType == AllowanceType.Relative ? 1e5 : votesToCast) |
There was a problem hiding this comment.
I think it's good to keep string/number repeateble values in the constants to keep code readable
e.g
uint constant public MAX_ALLOWANCE = 1e5;
| @@ -212,17 +165,16 @@ contract AlligatorOPV5 is IAlligatorOPV5, UUPSUpgradeable, OwnableUpgradeable, P | |||
| * @param support The support value for the vote. 0=against, 1=for, 2=abstain | |||
| */ | |||
| function castVoteBySig( | |||
| address[] calldata authority, | |||
There was a problem hiding this comment.
It looks like authority param still presented in the doc comments while deleted in the code
| @@ -235,7 +187,6 @@ contract AlligatorOPV5 is IAlligatorOPV5, UUPSUpgradeable, OwnableUpgradeable, P | |||
| * @param params The custom params of the vote | |||
| */ | |||
| function castVoteWithReasonAndParamsBySig( | |||
There was a problem hiding this comment.
It looks like authority param still presented in the doc comments while deleted in the code
| } | ||
| } | ||
|
|
||
| function _validateAndApplyRules(address from, SubdelegationRules rules, uint256 balance) { |
There was a problem hiding this comment.
this function has no identifier like: private, public, internal
| } else { | ||
| // Check if the address is already in the array | ||
| bool found = false; | ||
| for (uint256 i = 0; i < backwardSubdelegationKeys[from].length; i++) { |
There was a problem hiding this comment.
If we'll save backwardSubdelegationKeys[from].length to the local variable and use in the cycle it would be gas efficient. For instance I did some benchmark on 10-30 items:
This PR is WIP. It's an attempt to illustrate a potential algorithm for reading voting power directly from Alligator