refactor: comments in the angle-governance system (#2)

README.md

@@ -19,9 +19,9 @@ It also comes with some utils and scripts to facilitate the creation and executi
 
 ## System Architecture 🏘️
 
-Angle onchain governance works a![Alt text](../angle-governance-old/logo.svg) ../angle-governance-old/README.md ![Alt text](../angle-governance-old/DAO.png)s follows:
+Angle onchain governance works as follows:
 
-- veANGLE holders vote on Ethereum on an OpenZeppelin [`Governor`](contracts/AngleGovernor.sol) implementation called `AngleGovernor` with a predetermined quorum, voting delay and proposal threshold.
+- veANGLE holders vote on Ethereum on an OpenZeppelin [`Governor`](contracts/AngleGovernor.sol) implementation called `AngleGovernor` with a predetermined quorum, voting delay, proposal and shortcircuit thresholds.
 - On every chain where the protocol is deployed, there is a `Timelock` contract which is admin of all the protocol contracts (Borrowing module, Transmuter, direct deposit modules, ...) of its chain.
 - While only onchain votes can lead to payloads being included in the `Timelock` contract of a chain before execution, [Angle 4/6 Governance multisig](https://docs.angle.money/protocol-governance/angle-dao) (deployed on all chains as well) has a veto power on the payloads in Timelock contracts, and can cancel rogue governance votes.
 - For successful votes on non-Ethereum proposals, payloads to execute are bridged to the chain of interest using LayerZero message passing technology before being sent to the `Timelock` contract of their chain.
@@ -53,7 +53,7 @@ It's worth noting that, setup like this, the Angle Governance system can be abst
 
 ## Audits
 
-- The `AngleGovernor` implementation relies on several OpenZeppelin extensions as well as on the [audited](http://blog.openzeppelin.com/scopelift-flexible-voting-audit) [`GovernorCountingFractional` extension](https://github.com/ScopeLift/flexible-voting/blob/4399694c1a70d9e236c4c072802bfbe8e4951bf0/src/GovernorCountingFractional.sol) by ScopeLift.
+- The `AngleGovernor` implementation relies on several OpenZeppelin extensions, on the [audited](http://blog.openzeppelin.com/scopelift-flexible-voting-audit) [`GovernorCountingFractional` extension](https://github.com/ScopeLift/flexible-voting/blob/4399694c1a70d9e236c4c072802bfbe8e4951bf0/src/GovernorCountingFractional.sol) by ScopeLift. It is a fork of the [audited](https://github.com/trailofbits/publications/blob/master/reviews/2023-05-fraxgov-securityreview.pdf) [governance system by FRAX](https://github.com/FraxFinance/frax-governance).
 - The [`ProposalReceiver`](contracts/ProposalReceiver.sol) and [`ProposalSender`](contracts/ProposalSender.sol) contracts are forks from LayerZero Labs implementation. Find their audits [here](https://github.com/LayerZero-Labs/omnichain-governance-executor/tree/main/audits).
 
 ### Bug Bounty
@@ -144,7 +144,7 @@ You can run tests as follows:
 
 ```bash
 forge test -vvvv --watch
-forge test -vvvv --match-path contracts/forge-tests/KeeperMulticall.t.sol
+forge test -vvvv --match-path test/unit/Constants.t.sol
 forge test -vvvv --match-test "testAbc*"
 forge test -vvvv --fork-url https://eth-mainnet.alchemyapi.io/v2/Lc7oIGYeL_QvInzI0Wiu_pOZZDEKBrdf
 ```
@@ -206,7 +206,6 @@ For any question or feedback you can send an email to [[email protected]](mail
 
 This repository is released under the [MIT License](LICENSE).
 
-
 ## Media
 
 Don't hesitate to reach out on [Twitter](https://twitter.com/AngleProtocol) 🐦

contracts/AngleGovernor.sol

@@ -23,8 +23,9 @@ import "./utils/Errors.sol";
 /// @dev Core of Angle governance system, extending various OpenZeppelin modules
 /// @dev This contract overrides some OpenZeppelin function, like those in `GovernorSettings` to introduce
 /// the `onlyExecutor` modifier which ensures that only the Timelock contract can update the system's parameters
-/// @dev The time parameters (`votingDelay`, `votingPeriod`, ...) are expressed here in block number units which
-///  means that this implementation is only suited for an Ethereum deployment
+/// @dev The time parameters (`votingDelay`, `votingPeriod`, ...) are expressed here in timestamp units, but the
+/// also has a `votingDelayBlocks` parameters which must be set in accordance to the `votingDelay`
+/// @dev The `state` and `propose` functions here were forked from FRAX governance implementation
 /// @custom:security-contact [email protected]
 contract AngleGovernor is
     GovernorSettings,
@@ -123,7 +124,7 @@ contract AngleGovernor is
 
     /// @inheritdoc Governor
     // solhint-disable-next-line
-    /// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/
+    /// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/FraxGovernorAlpha.sol
     function state(uint256 proposalId) public view override returns (ProposalState) {
         // We read the struct fields into the stack at once so Solidity emits a single SLOAD
         ProposalCore storage proposal = _proposals[proposalId];
@@ -172,7 +173,7 @@ contract AngleGovernor is
 
     /// @inheritdoc Governor
     // solhint-disable-next-line
-    /// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/
+    /// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/FraxGovernorAlpha.sol
     function _propose(
         address[] memory targets,
         uint256[] memory values,

contracts/VeANGLEVotingDelegation.sol

@@ -10,10 +10,11 @@ import { IveANGLE } from "./interfaces/IveANGLE.sol";
 import { IveANGLEVotingDelegation } from "./interfaces/IveANGLEVotingDelegation.sol";
 
 /// @title VeANGLEVotingDelegation
+/// @notice Contract that keeps track of voting weights and delegations, leveraging veANGLE
 /// @author Jon Walch (Frax Finance) https://github.com/jonwalch
 // solhint-disable-next-line
-/// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/veANGLEVotingDelegation.sol
-/// @notice Contract that keeps track of voting weights and delegations, leveraging veANGLE
+/// @dev Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/veANGLEVotingDelegation.sol
+/// @dev The Fxs in the variable names and comments have been replaced by ANGLE
 contract VeANGLEVotingDelegation is EIP712, IERC5805 {
     using SafeCast for uint256;
 
@@ -54,8 +55,7 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         VE_ANGLE = IveANGLE(veANGLE);
     }
 
-    /// @notice The ```getCheckpoint``` function is called to retrieve a specific delegation checkpoint
-    /// @dev Get the checkpoint for a delegate at a given index.
+    /// @notice Retrieves a specific delegation checkpoint for a delegate at a given index
     /// @param delegateAddress Address of delegate
     /// @param index Integer index of the checkpoint
     /// @return delegateCheckpoint DelegateCheckpoint of ```delegate``` at ```index```
@@ -66,8 +66,8 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         return $delegateCheckpoints[delegateAddress][index];
     }
 
-    /// @notice The ```_calculateDelegatedWeight``` function calculates weight delegated to ```account```
-    /// accounting for any weight that expired since the nearest checkpoint
+    /// @notice Calculates weight delegated to ```account``` accounting for any weight that expired
+    /// since the nearest checkpoint
     /// @dev May include own weight if account previously delegated to someone else and then back to themselves
     /// @param voter Address of voter
     /// @param timestamp A block.timestamp, typically corresponding to a proposal snapshot
@@ -108,8 +108,8 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         delegatedWeight = expirationAdjustedAngle + (VOTE_WEIGHT_MULTIPLIER * biasAtTimestamp);
     }
 
-    /// @notice The ```_calculateVotingWeight``` function calculates ```account```'s voting weight.
-    /// Is 0 if they ever delegated and the delegation is in effect.
+    /// @notice Calculates ```account```'s voting weight.
+    /// @dev Is 0 if they ever delegated and the delegation is in effect.
     /// @param voter Address of voter
     /// @param timestamp A block.timestamp, typically corresponding to a proposal snapshot
     /// @return votingWeight Voting weight corresponding to ```account```'s veANGLE balance
@@ -209,22 +209,22 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
             _calculateDelegatedWeight({ voter: voter, timestamp: timestamp });
     }
 
-    /// @notice The ```getVotes``` function calculates a voter's weight at ```timestamp```
+    /// @notice Calculates a voter's weight at ```timestamp```
     /// @param voter Address of voter
     /// @param timepoint A block.timestamp, typically corresponding to a proposal snapshot
     /// @return votingWeight Voting weight of ```voterAddress``` at ```timepoint```
     function getVotes(address voter, uint256 timepoint) external view returns (uint256) {
         return _getVotingWeight({ voter: voter, timestamp: timepoint });
     }
 
-    /// @notice The ```getVotes``` function calculates a voter's weight at the current block.timestamp
+    /// @notice Calculates a voter's weight at the current block.timestamp
     /// @param voter Address of voter
     /// @return votingWeight Voting weight of ```voterAddress``` at ```block.timestamp```
     function getVotes(address voter) external view returns (uint256 votingWeight) {
         votingWeight = _getVotingWeight({ voter: voter, timestamp: block.timestamp });
     }
 
-    /// @notice The ```getPastVotes``` function calculates a voter's weight at ```timepoint```
+    /// @notice Calculates a voter's weight at ```timepoint```
     /// @param voter Address of voter
     /// @param timepoint A block.timestamp, typically corresponding to a proposal snapshot, must be in past
     /// @return pastVotingWeight Voting weight of ```account``` at ```timepoint``` in past
@@ -249,20 +249,20 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         pastTotalSupply = VE_ANGLE.totalSupplyAt(blockNumber);
     }
 
-    /// @notice The ```delegates``` function returns who the address of the delegate, that delegatorAddress has chosen
+    /// @notice Returns who the address of the delegate, that delegatorAddress has chosen
     /// @param delegator Address of delegator
     /// @return delegateAddress Address of the delegate
     function delegates(address delegator) external view returns (address delegateAddress) {
         delegateAddress = $delegations[delegator].delegate;
     }
 
-    /// @notice The ```delegate``` function delegates votes from signer to ```delegatee``` at the next epoch
+    /// @notice Delegates votes from signer to ```delegatee``` at the next epoch
     /// @param delegatee Address to delegate to
     function delegate(address delegatee) external {
         _delegate({ delegator: msg.sender, delegatee: delegatee });
     }
 
-    /// @notice The ```delegate``` function delegates votes from signer to ```delegatee```
+    /// @notice Delegates votes from signer to ```delegatee```
     /// @param delegatee Address to delegate to
     /// @param nonce Nonce of signed message
     /// @param expiry Expiry time of message
@@ -292,7 +292,7 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         _delegate(signer, delegatee);
     }
 
-    /// @notice The ```_delegate``` function delegates votes from signer to ```delegatee``` at the next epoch
+    /// @notice Delegates votes from signer to ```delegatee``` at the next epoch
     /// @param delegator Caller delegating their weight
     /// @param delegatee Address to delegate to
     /// @dev An account can only delegate to one account at a time. The previous delegation will be overwritten.
@@ -373,7 +373,7 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         normalizedVeANGLELockInfo.bias = SafeCast.toUint256(userBias) + normalizedVeANGLELockInfo.slope * lastUpdate;
     }
 
-    /// @notice Do a binary search for the closest checkpoint before ```timestamp```
+    /// @notice Does a binary search for the closest checkpoint before ```timestamp```
     /// @param _$checkpoints Storage pointer to the account's DelegateCheckpoints
     /// @param timestamp block.timestamp to get voting power at, frequently proposal snapshot
     /// @return closestCheckpoint The closest DelegateCheckpoint before timestamp
@@ -411,8 +411,8 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         closestCheckpoint = high == 0 ? closestCheckpoint : _$checkpoints[high - 1];
     }
 
-    /// @notice Removes voting power from the previous delegate, handling expirations
-    /// @notice and writing a new DelegateCheckpoint
+    /// @notice Removes voting power from the previous delegate, handling expirations and writing
+    /// a new DelegateCheckpoint
     /// @param previousDelegation The delegator's previous delegation
     /// @param checkpointTimestamp block.timestamp of the next DelegateCheckpoint's epoch
     function _moveVotingPowerFromPreviousDelegate(
@@ -488,8 +488,7 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         }
     }
 
-    /// @notice Add voting power to the new delegate, handling expirations
-    /// @notice and writing a new DelegateCheckpoint
+    /// @notice Adds voting power to the new delegate, handling expirations and writing a new DelegateCheckpoint
     /// @param newDelegate The new delegate that is being delegated to
     /// @param delegatorVeANGLELockInfo Information about the delegator's veANGLE lock
     /// @param checkpointTimestamp block.timestamp of the next DelegateCheckpoint's epoch
@@ -553,7 +552,7 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         });
     }
 
-    /// @notice Calculate the values to be written for the new DelegateCheckpoint
+    /// @notice Calculates the values to be written for the new DelegateCheckpoint
     /// @param previousCheckpoint The previous checkpoint for account
     /// @param account The account to calculate the expirations for
     /// @param isDeltaPositive Whether adding or subtracting from the previous checkpoint
@@ -627,8 +626,8 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         }
     }
 
-    /// @notice Push a new checkpoint to the array
-    /// or updates the most recent one if it already exists for the current epoch
+    /// @notice Pushes a new checkpoint to the array or updates the most recent one if it already
+    /// exists for the current epoch
     /// @param $userDelegationCheckpoints Pointer to the user's delegation checkpoints
     /// @param accountCheckpointsLength The length of the user's delegation checkpoints
     /// @param newCheckpoint The new checkpoint returned from _calculateCheckpoint()
@@ -648,8 +647,8 @@ contract VeANGLEVotingDelegation is EIP712, IERC5805 {
         }
     }
 
-    /// @notice Generate a summation of all bias, slope, and angle
-    /// for all delegations that expire during the specified time window for ```account```
+    /// @notice Generates a summation of all bias, slope, and angle for all delegations that expire during the
+    /// specified time window for ```account```
     /// @param account Delegate account to generate summations for
     /// @param start Timestamp to start the summations from. The start is not included
     /// @param end Timestamp to end the summations. The end is included

contracts/external/GovernorCountingFractional.sol

@@ -18,7 +18,10 @@ import "../utils/Errors.sol";
  * voting with tokens that are held by a DeFi pool, voting from L2 with tokens
  * held by a bridge, or voting privately from a shielded pool using zero
  * knowledge proofs.
+ * @author ScopeLift
  */
+//solhint-disable-next-line
+// Fork from: https://github.com/ScopeLift/flexible-voting/blob/4399694c1a70d9e236c4c072802bfbe8e4951bf0/src/GovernorCountingFractional.sol
 abstract contract GovernorCountingFractional is Governor {
     struct ProposalVote {
         uint128 againstVotes;