From e86fb5ae1c1c08dc46dcbceed5539eaf06176073 Mon Sep 17 00:00:00 2001 From: Pablo Veyrat Date: Mon, 20 Nov 2023 20:18:09 +0100 Subject: [PATCH] wip: pablo review --- contracts/AngleGovernor.sol | 50 +++++++++++++++---- contracts/VeANGLEVotingDelegation.sol | 2 +- .../external/GovernorCountingFractional.sol | 20 +++----- contracts/external/GovernorShortCircuit.sol | 3 +- contracts/external/GovernorToken.sol | 6 +-- 5 files changed, 52 insertions(+), 29 deletions(-) diff --git a/contracts/AngleGovernor.sol b/contracts/AngleGovernor.sol index 8953a6b..eb9e78d 100644 --- a/contracts/AngleGovernor.sol +++ b/contracts/AngleGovernor.sol @@ -20,11 +20,10 @@ import "./utils/Errors.sol"; /// @title AngleGovernor /// @author Angle Labs, Inc /// @dev Core of Angle governance system, extending various OpenZeppelin modules -/// @dev This contract overrides some OpenZeppelin function, like those in `GovernorSettings` to introduce +/// @dev This contract overrides some OpenZeppelin functions, 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 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 +/// contract also has a `votingDelayBlocks` parameter which must be set in accordance to the `votingDelay` /// @custom:security-contact contact@angle.money contract AngleGovernor is GovernorSettings, @@ -150,15 +149,32 @@ contract AngleGovernor is /// @inheritdoc Governor // solhint-disable-next-line - /// @notice Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/FraxGovernorAlpha.sol + /// @notice Taken from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/FraxGovernorAlpha.sol function state(uint256 proposalId) public view override returns (ProposalState) { - ProposalState classicProposalState = Governor.state(proposalId); + // We read the struct fields into the stack at once so Solidity emits a single SLOAD + ProposalCore storage proposal = _proposals[proposalId]; + bool proposalExecuted = proposal.executed; + bool proposalCanceled = proposal.canceled; + + if (proposalExecuted) { + return ProposalState.Executed; + } + + if (proposalCanceled) { + return ProposalState.Canceled; + } + + uint256 snapshot = proposalSnapshot(proposalId); + + if (snapshot == 0) { + revert GovernorNonexistentProposal(proposalId); + } - if ( - classicProposalState == ProposalState.Executed || - classicProposalState == ProposalState.Canceled || - classicProposalState == ProposalState.Pending - ) return classicProposalState; + uint256 currentTimepoint = clock(); + + if (snapshot >= currentTimepoint || $snapshotTimestampToSnapshotBlockNumber[snapshot] >= block.number) { + return ProposalState.Pending; + } // Allow early execution when overwhelming majority (bool isShortCircuitFor, bool isShortCircuitAgainst) = _shortCircuit(proposalId); @@ -166,7 +182,19 @@ contract AngleGovernor is return ProposalState.Succeeded; } else if (isShortCircuitAgainst) { return ProposalState.Defeated; - } else return classicProposalState; + } + + uint256 deadline = proposalDeadline(proposalId); + + if (deadline >= currentTimepoint) { + return ProposalState.Active; + } else if (!_quorumReached(proposalId) || !_voteSucceeded(proposalId)) { + return ProposalState.Defeated; + } else if (proposalEta(proposalId) == 0) { + return ProposalState.Succeeded; + } else { + return ProposalState.Queued; + } } /// @inheritdoc GovernorVotesQuorumFraction diff --git a/contracts/VeANGLEVotingDelegation.sol b/contracts/VeANGLEVotingDelegation.sol index 2df7792..162909d 100644 --- a/contracts/VeANGLEVotingDelegation.sol +++ b/contracts/VeANGLEVotingDelegation.sol @@ -13,7 +13,7 @@ import { IveANGLEVotingDelegation } from "./interfaces/IveANGLEVotingDelegation. /// @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 -/// @dev Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/veANGLEVotingDelegation.sol +/// @dev Fork from Frax Finance: https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/VeFxsVotingDelegation.sol /// @dev The Fxs in the variable names and comments have been replaced by ANGLE contract VeANGLEVotingDelegation is EIP712, IERC5805 { using SafeCast for uint256; diff --git a/contracts/external/GovernorCountingFractional.sol b/contracts/external/GovernorCountingFractional.sol index af68dec..c03fbbc 100644 --- a/contracts/external/GovernorCountingFractional.sol +++ b/contracts/external/GovernorCountingFractional.sol @@ -8,18 +8,14 @@ import { SafeCast } from "oz/utils/math/SafeCast.sol"; import "../utils/Errors.sol"; -/** - * @notice Extension of {Governor} for 3 option fractional vote counting. When - * voting, a delegate may split their vote weight between Against/For/Abstain. - * This is most useful when the delegate is itself a contract, implementing its - * own rules for voting. By allowing a contract-delegate to split its vote - * weight, the voting preferences of many disparate token holders can be rolled - * up into a single vote to the Governor itself. Some example use cases include - * 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 - */ +/// @title GovernorCountingFractional +/// @notice Extension of {Governor} for 3 option fractional vote counting. When voting, a delegate may split their +/// vote weight between Against/For/Abstain. This is most useful when the delegate is itself a contract, implementing +/// its own rules for voting. By allowing a contract-delegate to split its vote weight, the voting preferences of many +/// disparate token holders can be rolled up into a single vote to the Governor itself. Some example use cases include +/// 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 { diff --git a/contracts/external/GovernorShortCircuit.sol b/contracts/external/GovernorShortCircuit.sol index edeadad..a200d6b 100644 --- a/contracts/external/GovernorShortCircuit.sol +++ b/contracts/external/GovernorShortCircuit.sol @@ -12,10 +12,9 @@ import "../utils/Errors.sol"; /// @title GovernorShortCircuit /// @notice Extends governor to pass propositions if the quorum is reached before the end of the voting period -/// @author Angle Labs, Inc /// @author Jon Walch (Frax Finance) https://github.com/jonwalch //solhint-disable-next-line -/// @notice https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/veANGLEVotingDelegation.sol +/// @notice https://github.com/FraxFinance/frax-governance/blob/e465513ac282aa7bfd6744b3136354fae51fed3c/src/VeFxsVotingDelegation.sol abstract contract GovernorShortCircuit is GovernorVotes, GovernorCountingFractional, GovernorVotesQuorumFraction { using SafeCast for *; using Checkpoints for Checkpoints.Trace224; diff --git a/contracts/external/GovernorToken.sol b/contracts/external/GovernorToken.sol index 62c19ae..308269d 100644 --- a/contracts/external/GovernorToken.sol +++ b/contracts/external/GovernorToken.sol @@ -5,9 +5,9 @@ pragma solidity ^0.8.20; import { IVotes } from "oz/governance/utils/IVotes.sol"; import { GovernorVotes, IERC5805 } from "oz/governance/extensions/GovernorVotes.sol"; -/** - * @notice Extension of {Governor} with `internal` and not `private` _token - */ +/// @title GovernorToken +/// @author Angle Labs, Inc. +/// @notice Extension of {Governor} with an `internal` (and not `private`) _token_ variable which can be modified abstract contract GovernorToken is GovernorVotes { event VeANGLEVotingDelegationSet(address oldVotingDelegation, address newVotingDelegation);