audit: blocksec comments II

audits/AUDIT_REPORT.md

@@ -96,3 +96,13 @@ Aragon Audits were conducted by various team members.
 | U        | Internal Review: x.pow(2) is less precise and more expensive than x.mul(x)                | Fixed        | https://github.com/aragon/ve-governance/pull/12                                                                                     |
 | U        | Internal Review: passing very small weights to gauges leads to precision loss             | Acknowledged | In this build, voting is signalling only, so rounding errors are ok for small values, we may wish to reevaluate with onchain voting |
 | U        | Internal Review: Small deposits and fees will round to zero in the exit queue             | Acknowledged | Fee rounding to zero simply won't charge a fee, we don't think it's a major issue + we can enforce a min deposit                    |
+
+# Notes relating to the second round of audits by BlockSec
+
+| Severity     | Issue                                                                                                                                                                                                                                                                                                                                                                                         | Status       | Comment or PR                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
+| ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| U            | New Note-1: Effective voting power after the invocation of function beginWithdrawal()                                                                                                                                                                                                                                                                                                         |
+| Acknowledged | This is correct and is acknowledged in the same manner as HAL-12. The voting power of votes in the withdrawal queue is active until the end of the current interval, but it cannot be used as the NFT is held by the escrow, so in the current implementation we don't believe this is a problem. Blocksec is correct to acknowledge that this should be addressed in future implementations. |
+| U            | New Note-2                                                                                                                                                                                                                                                                                                                                                                                    | Acknowledged | Allowing the users to change votes during the voting period is intended behaviour in this implementation.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                           |
+| U            | New Question-1                                                                                                                                                                                                                                                                                                                                                                                | Acknowledged | The purpose of the warmup period is to function as a minimum buffer before users can vote. In the current implementation, voting power starts accumulating from the start of the upcoming deposit interval (weekly by default), but to avoid someone opportunisitically locking 1 second before voting starts, we add a minimum period where they cannot vote. The intended behaviour is that the voting power _should_ still accumulate during this time.                                                                                                                                                                                                                                                                          |
+| U            | New Question-2                                                                                                                                                                                                                                                                                                                                                                                | TBC          | Reviewing this in depth I believe the behaviour you mentioned is intended. The resolveEpochVoteXXIn function uses the sentinel value "0" to indicate "has started" (startsIn) or "has ended" (endsIn), else it returns a positive value. In the example you give, if one is outside the voting window. `EndsIn` should return 0 and `StartsIn` should return the seconds until the next voting window (including the buffer). That said, I agree this needs to be approached with care, the above semantics may not be well understood, and perhaps a different way of expressing active should be fetched. It's telling that the `resolveVotingActive` doesn't use the zero sentinel value, so again, it's not particularly clear. |