Skip to content

Commit

Permalink
feat: lock to propose (#9430)
Browse files Browse the repository at this point in the history
Fixes #9348
  • Loading branch information
LHerskind authored Nov 6, 2024
1 parent 4c51c75 commit 538cd47
Show file tree
Hide file tree
Showing 14 changed files with 262 additions and 42 deletions.
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),
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.
*
* @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);
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

0 comments on commit 538cd47

Please sign in to comment.