Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 72 additions & 28 deletions l1-contracts/src/governance/Apella.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ contract Apella is IApella {
gerousia = _gerousia;

configuration = DataStructures.Configuration({
proposeConfig: DataStructures.ProposeConfiguration({
lockDelay: Timestamp.wrap(3600),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make an issue to update the delays here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lockAmount: 1000e18
}),
votingDelay: Timestamp.wrap(3600),
votingDuration: Timestamp.wrap(3600),
executionDelay: Timestamp.wrap(3600),
Expand Down Expand Up @@ -81,21 +85,7 @@ contract Apella is IApella {
override(IApella)
returns (uint256)
{
users[msg.sender].sub(_amount);
total.sub(_amount);

uint256 withdrawalId = withdrawalCount++;

withdrawals[withdrawalId] = DataStructures.Withdrawal({
amount: _amount,
unlocksAt: Timestamp.wrap(block.timestamp) + configuration.lockDelay(),
recipient: _to,
claimed: false
});

emit WithdrawInitiated(withdrawalId, _to, _amount);

return withdrawalId;
return _initiateWithdraw(_to, _amount, configuration.withdrawalDelay());
}

function finaliseWithdraw(uint256 _withdrawalId) external override(IApella) {
Expand All @@ -114,21 +104,37 @@ contract Apella is IApella {

function propose(IPayload _proposal) external override(IApella) returns (bool) {
require(msg.sender == gerousia, Errors.Apella__CallerNotGerousia(msg.sender, gerousia));
return _propose(_proposal);
}

uint256 proposalId = proposalCount++;

proposals[proposalId] = DataStructures.Proposal({
config: configuration,
state: DataStructures.ProposalState.Pending,
payload: _proposal,
creator: msg.sender,
creation: Timestamp.wrap(block.timestamp),
summedBallot: DataStructures.Ballot({yea: 0, nea: 0})
});
/**
* @notice Propose a new proposal by locking up a bunch of power
*
* Beware that if the gerousia changes these proposals will also be dropped
* This is to ensure consistency around way proposals are made, and they should
* really be using the proposal logic in Gerousia, which might have a similar
* mechanism in place as well.
* It is here for emergency purposes.
* Using the lock should be a last resort if the Gerousia is broken.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention that gerousia is broken if no proposals are made through it, its not immediately clear that this means validators are no longer able to make proposals

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Gerousia" can also be seen as broken if it is not agreeing with what token holders want, so it is also a way to bypass it 🤷.

*
* @param _proposal The proposal to propose
* @param _to The address to send the lock to
* @return True if the proposal was proposed
*/
function proposeWithLock(IPayload _proposal, address _to)
external
override(IApella)
returns (bool)
{
uint256 availablePower = users[msg.sender].powerNow();
uint256 amount = configuration.proposeConfig.lockAmount;

emit Proposed(proposalId, address(_proposal));
require(
amount <= availablePower, Errors.Apella__InsufficientPower(msg.sender, availablePower, amount)
);

return true;
_initiateWithdraw(_to, amount, configuration.proposeConfig.lockDelay);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can make it more explicit that this delay is longer than votingDelay + votingDuration to prevent someone from voting on their own emergency proposal. Right now this is the same delay as any other withdrawal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not the same delay as any other withdrawal, have renamed it to make it more clear as I could see the confusion.

return _propose(_proposal);
}

function vote(uint256 _proposalId, uint256 _amount, bool _support)
Expand Down Expand Up @@ -264,7 +270,7 @@ contract Apella is IApella {
}

// If the gerousia have changed we mark is as dropped
if (gerousia != self.creator) {
if (gerousia != self.gerousia) {
return DataStructures.ProposalState.Dropped;
}

Expand Down Expand Up @@ -294,4 +300,42 @@ contract Apella is IApella {

return DataStructures.ProposalState.Expired;
}

function _initiateWithdraw(address _to, uint256 _amount, Timestamp _delay)
internal
returns (uint256)
{
users[msg.sender].sub(_amount);
total.sub(_amount);

uint256 withdrawalId = withdrawalCount++;

withdrawals[withdrawalId] = DataStructures.Withdrawal({
amount: _amount,
unlocksAt: Timestamp.wrap(block.timestamp) + _delay,
recipient: _to,
claimed: false
});

emit WithdrawInitiated(withdrawalId, _to, _amount);

return withdrawalId;
}

function _propose(IPayload _proposal) internal returns (bool) {
uint256 proposalId = proposalCount++;

proposals[proposalId] = DataStructures.Proposal({
config: configuration,
state: DataStructures.ProposalState.Pending,
payload: _proposal,
gerousia: gerousia,
creation: Timestamp.wrap(block.timestamp),
summedBallot: DataStructures.Ballot({yea: 0, nea: 0})
});

emit Proposed(proposalId, address(_proposal));

return true;
}
}
1 change: 1 addition & 0 deletions l1-contracts/src/governance/interfaces/IApella.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ interface IApella {
function initiateWithdraw(address _to, uint256 _amount) external returns (uint256);
function finaliseWithdraw(uint256 _withdrawalId) external;
function propose(IPayload _proposal) external returns (bool);
function proposeWithLock(IPayload _proposal, address _to) external returns (bool);
function vote(uint256 _proposalId, uint256 _amount, bool _support) external returns (bool);
function execute(uint256 _proposalId) external returns (bool);
function dropProposal(uint256 _proposalId) external returns (bool);
Expand Down
22 changes: 21 additions & 1 deletion l1-contracts/src/governance/libraries/ConfigurationLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ library ConfigurationLib {
Timestamp internal constant TIME_LOWER = Timestamp.wrap(3600);
Timestamp internal constant TIME_UPPER = Timestamp.wrap(30 * 24 * 3600);

function lockDelay(DataStructures.Configuration storage _self) internal view returns (Timestamp) {
function withdrawalDelay(DataStructures.Configuration storage _self)
internal
view
returns (Timestamp)
{
return Timestamp.wrap(Timestamp.unwrap(_self.votingDelay) / 5) + _self.votingDuration
+ _self.executionDelay;
}
Expand All @@ -40,6 +44,22 @@ library ConfigurationLib {
require(
_self.minimumVotes >= VOTES_LOWER, Errors.Apella__ConfigurationLib__InvalidMinimumVotes()
);
require(
_self.proposeConfig.lockAmount >= VOTES_LOWER,
Errors.Apella__ConfigurationLib__LockAmountTooSmall()
);

// Beyond checking the bounds like this, it might be useful to ensure that the value is larger than the withdrawal delay
// this, can be useful if one want to ensure that the "locker" cannot himself vote in the proposal, but as it is unclear
// if this is a useful property, it is not enforced.
require(
_self.proposeConfig.lockDelay >= TIME_LOWER,
Errors.Apella__ConfigurationLib__TimeTooSmall("LockDelay")
);
require(
_self.proposeConfig.lockDelay <= TIME_UPPER,
Errors.Apella__ConfigurationLib__TimeTooBig("LockDelay")
);

require(
_self.votingDelay >= TIME_LOWER, Errors.Apella__ConfigurationLib__TimeTooSmall("VotingDelay")
Expand Down
8 changes: 7 additions & 1 deletion l1-contracts/src/governance/libraries/DataStructures.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@ library DataStructures {
}
// docs:end:registry_snapshot

struct ProposeConfiguration {
Timestamp lockDelay;
uint256 lockAmount;
}

struct Configuration {
ProposeConfiguration proposeConfig;
Timestamp votingDelay;
Timestamp votingDuration;
Timestamp executionDelay;
Expand Down Expand Up @@ -53,7 +59,7 @@ library DataStructures {
Configuration config;
ProposalState state;
IPayload payload;
address creator;
address gerousia;
Timestamp creation;
Ballot summedBallot;
}
Expand Down
1 change: 1 addition & 0 deletions l1-contracts/src/governance/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ library Errors {
error Apella__UserLib__NotInPast();

error Apella__ConfigurationLib__InvalidMinimumVotes();
error Apella__ConfigurationLib__LockAmountTooSmall();
error Apella__ConfigurationLib__QuorumTooSmall();
error Apella__ConfigurationLib__QuorumTooBig();
error Apella__ConfigurationLib__DifferentialTooSmall();
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/governance/apella/base.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ contract ApellaBase is TestBase {
proposal = proposals[_proposalName];
proposalId = proposalIds[_proposalName];

vm.assume(_gerousia != proposal.creator);
vm.assume(_gerousia != proposal.gerousia);

vm.prank(address(apella));
apella.updateGerousia(_gerousia);
Expand Down
8 changes: 5 additions & 3 deletions l1-contracts/test/governance/apella/getProposalState.t.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.27;

import {stdStorage, StdStorage} from "forge-std/Test.sol";

import {ApellaBase} from "./base.t.sol";
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";
import {Errors} from "@aztec/governance/libraries/Errors.sol";
Expand All @@ -9,6 +11,7 @@ import {ProposalLib, VoteTabulationReturn} from "@aztec/governance/libraries/Pro

contract GetProposalStateTest is ApellaBase {
using ProposalLib for DataStructures.Proposal;
using stdStorage for StdStorage;

function test_WhenProposalIsOutOfBounds(uint256 _index) external {
// it revert
Expand Down Expand Up @@ -153,9 +156,8 @@ contract GetProposalStateTest is ApellaBase {

// We can overwrite the quorum to be 0 to hit an invalid case
assertGt(apella.getProposal(proposalId).config.quorum, 0);
bytes32 slot =
bytes32(uint256(keccak256(abi.encodePacked(uint256(proposalId), uint256(1)))) + 4);
vm.store(address(apella), slot, 0);
stdstore.target(address(apella)).sig("getProposal(uint256)").with_key(proposalId).depth(6)
.checked_write(uint256(0));
assertEq(apella.getProposal(proposalId).config.quorum, 0);

uint256 totalPower = apella.totalPowerAt(Timestamp.wrap(block.timestamp));
Expand Down
3 changes: 1 addition & 2 deletions l1-contracts/test/governance/apella/initiateWithdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ pragma solidity >=0.8.27;

import {ApellaBase} from "./base.t.sol";
import {IApella} from "@aztec/governance/interfaces/IApella.sol";
import {IERC20Errors} from "@oz/interfaces/draft-IERC6093.sol";
import {Timestamp} from "@aztec/core/libraries/TimeMath.sol";
import {Errors} from "@aztec/governance/libraries/Errors.sol";
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";
Expand Down Expand Up @@ -88,7 +87,7 @@ contract InitiateWithdrawTest is ApellaBase {
assertEq(withdrawal.amount, amount, "invalid amount");
assertEq(
withdrawal.unlocksAt,
Timestamp.wrap(block.timestamp) + config.lockDelay(),
Timestamp.wrap(block.timestamp) + config.withdrawalDelay(),
"Invalid timestamp"
);
assertEq(withdrawal.recipient, recipient, "invalid recipient");
Expand Down
4 changes: 1 addition & 3 deletions l1-contracts/test/governance/apella/propose.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ pragma solidity >=0.8.27;
import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
import {ApellaBase} from "./base.t.sol";
import {IApella} from "@aztec/governance/interfaces/IApella.sol";
import {IERC20Errors} from "@oz/interfaces/draft-IERC6093.sol";
import {Timestamp} from "@aztec/core/libraries/TimeMath.sol";
import {Errors} from "@aztec/governance/libraries/Errors.sol";
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";
import {ConfigurationLib} from "@aztec/governance/libraries/ConfigurationLib.sol";

contract ProposeTest is ApellaBase {
function test_WhenCallerIsNotGerousia() external {
Expand Down Expand Up @@ -45,7 +43,7 @@ contract ProposeTest is ApellaBase {
assertEq(proposal.config.votingDelay, config.votingDelay);
assertEq(proposal.config.votingDuration, config.votingDuration);
assertEq(proposal.creation, Timestamp.wrap(block.timestamp));
assertEq(proposal.creator, address(gerousia));
assertEq(proposal.gerousia, address(gerousia));
assertEq(proposal.summedBallot.nea, 0);
assertEq(proposal.summedBallot.yea, 0);
assertTrue(proposal.state == DataStructures.ProposalState.Pending);
Expand Down
55 changes: 55 additions & 0 deletions l1-contracts/test/governance/apella/proposeWithLock.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.27;

import {IPayload} from "@aztec/governance/interfaces/IPayload.sol";
import {ApellaBase} from "./base.t.sol";
import {IApella} from "@aztec/governance/interfaces/IApella.sol";
import {Timestamp} from "@aztec/core/libraries/TimeMath.sol";
import {Errors} from "@aztec/governance/libraries/Errors.sol";
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";

contract ProposeWithLockTest is ApellaBase {
function test_WhenCallerHasInsufficientPower() external {
// it revert
DataStructures.Configuration memory config = apella.getConfiguration();
vm.expectRevert(
abi.encodeWithSelector(
Errors.Apella__InsufficientPower.selector, address(this), 0, config.proposeConfig.lockAmount
)
);
apella.proposeWithLock(IPayload(address(0)), address(this));
}

function test_WhenCallerHasSufficientPower(address _proposal) external {
// it creates a withdrawal with the lock amount and delay
// it creates a new proposal with current config
// it emits a {ProposalCreated} event
// it returns true
DataStructures.Configuration memory config = apella.getConfiguration();
token.mint(address(this), config.proposeConfig.lockAmount);

token.approve(address(apella), config.proposeConfig.lockAmount);
apella.deposit(address(this), config.proposeConfig.lockAmount);

proposalId = apella.proposalCount();

vm.expectEmit(true, true, true, true, address(apella));
emit IApella.Proposed(proposalId, _proposal);

assertTrue(apella.proposeWithLock(IPayload(_proposal), address(this)));

DataStructures.Proposal memory proposal = apella.getProposal(proposalId);
assertEq(proposal.config.executionDelay, config.executionDelay);
assertEq(proposal.config.gracePeriod, config.gracePeriod);
assertEq(proposal.config.minimumVotes, config.minimumVotes);
assertEq(proposal.config.quorum, config.quorum);
assertEq(proposal.config.voteDifferential, config.voteDifferential);
assertEq(proposal.config.votingDelay, config.votingDelay);
assertEq(proposal.config.votingDuration, config.votingDuration);
assertEq(proposal.creation, Timestamp.wrap(block.timestamp));
assertEq(proposal.gerousia, address(gerousia));
assertEq(proposal.summedBallot.nea, 0);
assertEq(proposal.summedBallot.yea, 0);
assertTrue(proposal.state == DataStructures.ProposalState.Pending);
}
}
8 changes: 8 additions & 0 deletions l1-contracts/test/governance/apella/proposeWithLock.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ProposeWithLockTest
├── when caller has insufficient power
│ └── it revert
└── when caller has sufficient power
├── it creates a withdrawal with the lock amount and delay
├── it creates a new proposal with current config
├── it emits a {ProposalCreated} event
└── it returns true
Loading