Skip to content

Commit 3151616

Browse files
heueristikheueristik
committed
refactor: combine criteria
1 parent e279011 commit 3151616

File tree

6 files changed

+117
-80
lines changed

6 files changed

+117
-80
lines changed

‎src/XanV1.sol‎

Lines changed: 59 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ contract XanV1 is
5656
error UnlockedBalanceInsufficient(address sender, uint256 unlockedBalance, uint256 valueToLock);
5757
error LockedBalanceInsufficient(address sender, uint256 lockedBalance);
5858
error NoVotesToRevoke(address sender, address proposedImpl);
59+
5960
error ImplementationZero();
6061
error ImplementationAlreadyProposed(address impl);
6162
error ImplementationRankNonExistent(uint48 limit, uint48 rank);
@@ -67,10 +68,8 @@ contract XanV1 is
6768
error UpgradeCancellationInvalid(address impl, uint48 endTime);
6869
error UpgradeIsNotAllowedToBeScheduledTwice(address impl);
6970

70-
error MinLockedSupplyNotReached();
71-
error QuorumNowhereReached();
72-
error QuorumNotReached(address impl);
73-
error QuorumReached(address impl);
71+
error QuorumOrMinLockedSupplyNotReached(address impl);
72+
error QuorumAndMinLockedSupplyReached(address impl);
7473

7574
error DelayPeriodNotStarted(uint48 endTime);
7675
error DelayPeriodNotEnded(uint48 endTime);
@@ -192,16 +191,11 @@ contract XanV1 is
192191
revert UpgradeAlreadyScheduled(votingData.scheduledImpl, votingData.scheduledEndTime);
193192
}
194193

195-
// Check if the minimal locked supply is reached.
196-
if (!_isMinLockedSupplyReached()) {
197-
revert MinLockedSupplyNotReached();
198-
}
199-
200194
// Check if the proposed implementation is the best ranked implementation
201195
{
202-
address bestRankedVoterBodyImpl = votingData.ranking[0];
203-
if (!_isQuorumReached(bestRankedVoterBodyImpl)) {
204-
revert QuorumNotReached(bestRankedVoterBodyImpl);
196+
address bestRankedVoterBodyImpl = votingData.implementationByRank(0);
197+
if (!_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
198+
revert QuorumOrMinLockedSupplyNotReached(bestRankedVoterBodyImpl);
205199
}
206200

207201
// Schedule the upgrade and emit the associated event.
@@ -230,8 +224,14 @@ contract XanV1 is
230224

231225
// TODO! check min quorum
232226

233-
// Check that the quorum for the new implementation is reached.
234-
if (_isQuorumReached(data.scheduledImpl) && data.scheduledImpl == data.ranking[0]) {
227+
// Revert the cancellation if the currently scheduled implementation still
228+
// * meets the quorum and minimum locked supply for the
229+
// * is the best ranked implementation
230+
// and revert the cancellation if this is the case.
231+
if (
232+
_isQuorumAndMinLockedSupplyReached(data.scheduledImpl)
233+
&& (data.scheduledImpl == data.implementationByRank(0))
234+
) {
235235
revert UpgradeCancellationInvalid(data.scheduledImpl, data.scheduledEndTime);
236236
}
237237

@@ -243,26 +243,27 @@ contract XanV1 is
243243
}
244244

245245
/// @notice @inheritdoc IXanV1
246-
function scheduleCouncilUpgrade(address proposedImpl) external override onlyCouncil {
247-
// TODO! replace by `isAcceptedVoterBodyUpgrade`
246+
function scheduleCouncilUpgrade(address impl) external override onlyCouncil {
247+
// Check if a voter body upgrade could be scheduled.
248248
{
249249
Voting.Data storage votingData = _getVotingData();
250250

251-
address bestRankedVoterBodyImpl = votingData.ranking[0];
251+
address bestRankedVoterBodyImpl = votingData.implementationByRank(0);
252252

253-
if (_isQuorumReached(bestRankedVoterBodyImpl) && _isMinLockedSupplyReached()) {
254-
revert QuorumReached(bestRankedVoterBodyImpl);
253+
if (_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
254+
revert QuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl);
255255
}
256-
// TODO! add min quorum check
257256
}
258257

259258
Council.Data storage data = _getCouncilData();
260259

261-
if (data.scheduledImpl == proposedImpl) {
262-
revert ImplementationAlreadyProposed(proposedImpl);
260+
// Check if the council upgrade is already scheduled
261+
if (data.scheduledImpl != address(0) && data.scheduledEndTime != 0) {
262+
revert UpgradeAlreadyScheduled(data.scheduledImpl, data.scheduledEndTime);
263263
}
264264

265-
data.scheduledImpl = proposedImpl;
265+
// Schedule the council upgrade
266+
data.scheduledImpl = impl;
266267
data.scheduledEndTime = Time.timestamp() + Parameters.DELAY_DURATION;
267268

268269
emit CouncilUpgradeScheduled(data.scheduledImpl, data.scheduledEndTime);
@@ -276,14 +277,17 @@ contract XanV1 is
276277

277278
/// @notice @inheritdoc IXanV1
278279
function vetoCouncilUpgrade() external override {
279-
// Get the implementation with the most votes.
280-
address mostVotedImplementation = _getVotingData().ranking[0];
280+
// Get the best ranked implementation.
281+
address bestRankedImplementation = _getVotingData().implementationByRank(0);
282+
if (bestRankedImplementation == address(0)) {
283+
revert ImplementationRankNonExistent({limit: 0, rank: 0});
284+
}
281285

282286
// Check if the most voted implementation has reached quorum.
283-
if (!_isQuorumReached(mostVotedImplementation)) {
287+
if (!_isQuorumAndMinLockedSupplyReached(bestRankedImplementation)) {
284288
// The voter body has not reached quorum on any implementation.
285-
// This means that vetoing the council is not possible.
286-
revert QuorumNowhereReached();
289+
// This means that vetoing the council is not allowed.
290+
revert QuorumOrMinLockedSupplyNotReached(bestRankedImplementation);
287291
}
288292

289293
emit CouncilUpgradeVetoed();
@@ -297,6 +301,16 @@ contract XanV1 is
297301
votes = _getVotingData().ballots[proposedImpl].vota[msg.sender];
298302
}
299303

304+
/// @notice @inheritdoc IXanV1
305+
function proposedImplementationByRank(uint48 rank) external view override returns (address impl) {
306+
Voting.Data storage data = _getVotingData();
307+
308+
impl = data.implementationByRank(rank);
309+
if (impl == address(0)) {
310+
revert ImplementationRankNonExistent({limit: data.implCount, rank: rank});
311+
}
312+
}
313+
300314
/// @notice @inheritdoc IXanV1
301315
function lockedSupply() public view override returns (uint256 locked) {
302316
locked = _getLockingData().lockedSupply;
@@ -317,18 +331,6 @@ contract XanV1 is
317331
thisImplementation = ERC1967Utils.getImplementation();
318332
}
319333

320-
/// @notice @inheritdoc IXanV1
321-
function proposedImplementationByRank(uint48 rank) public view override returns (address rankedImplementation) {
322-
Voting.Data storage $ = _getVotingData();
323-
uint48 implCount = $.implCount;
324-
325-
if (implCount == 0 || rank > implCount - 1) {
326-
revert ImplementationRankNonExistent({limit: implCount, rank: rank});
327-
}
328-
329-
rankedImplementation = $.ranking[rank];
330-
}
331-
332334
/// @inheritdoc IXanV1
333335
function scheduledVoterBodyUpgrade() public view override returns (address impl, uint48 endTime) {
334336
Voting.Data storage data = _getVotingData();
@@ -420,37 +422,26 @@ contract XanV1 is
420422
revert UpgradeIsNotAllowedToBeScheduledTwice(newImpl);
421423
}
422424

423-
address bestRankedVoterBodyImpl = votingData.ranking[0];
425+
address bestRankedVoterBodyImpl = votingData.implementationByRank(0);
424426

425427
if (isScheduledByVoterBody) {
426428
if (newImpl != bestRankedVoterBodyImpl) {
427429
revert ImplementationNotRankedBest({expected: bestRankedVoterBodyImpl, actual: newImpl});
428430
}
429431

430-
if (!_isMinLockedSupplyReached()) {
431-
revert MinLockedSupplyNotReached();
432-
}
433-
// TODO combine `_isMinLockedSupplyReached` with the check below?
434-
// NOTE: It should not be possible to schedule an implementation without minLockedSupply
435-
// Check that the quorum for the new implementation is reached.
436-
if (!_isQuorumReached(bestRankedVoterBodyImpl)) {
437-
revert QuorumNotReached(bestRankedVoterBodyImpl);
432+
if (!_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
433+
revert QuorumOrMinLockedSupplyNotReached(bestRankedVoterBodyImpl);
438434
}
439-
440435
_checkDelayCriterion({endTime: votingData.scheduledEndTime});
441-
//_checkVoterBodyUpgradeCriteria({bestRankedVoterBodyImpl: bestRankedVoterBodyImpl});
442436

443437
return;
444438
}
445439

446440
if (isScheduledByCouncil) {
447-
// there cannot be ANY accepted voter body upgrade
448-
// TODO!
449-
450-
// Revert if the quorum is reached for the
451-
if (_isQuorumReached(bestRankedVoterBodyImpl) && _isMinLockedSupplyReached()) {
452-
// TODO! better error name
453-
revert QuorumReached(bestRankedVoterBodyImpl);
441+
// Revert if the quorum and minimum locked supply is reached for best ranked implementation proposed by
442+
// the voter body.
443+
if (_isQuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl)) {
444+
revert QuorumAndMinLockedSupplyReached(bestRankedVoterBodyImpl);
454445
}
455446

456447
_checkDelayCriterion({endTime: councilData.scheduledEndTime});
@@ -468,17 +459,17 @@ contract XanV1 is
468459
}
469460
}
470461

471-
/// @notice Returns `true` if the quorum is reached for a given mplementation.
462+
/// @notice Returns `true` if the quorum and minimum locked supply is reached for a given mplementation.
472463
/// @param impl The implementation to check the quorum criteria for.
473-
/// @return isReached Whether the quorum is reached or not.
474-
function _isQuorumReached(address impl) internal view returns (bool isReached) {
475-
isReached = totalVotes(impl) > calculateQuorumThreshold();
476-
}
477-
478-
/// @notice Returns `true` if the quorum is reached for a particular implementation.
479-
/// @return isReached Whether the minimum locked supply is reached or not.
480-
function _isMinLockedSupplyReached() internal view returns (bool isReached) {
481-
isReached = lockedSupply() + 1 > Parameters.MIN_LOCKED_SUPPLY;
464+
/// @return isReached Whether the quorum and minimum locked supply is reached or not.
465+
function _isQuorumAndMinLockedSupplyReached(address impl) internal view returns (bool isReached) {
466+
if (totalVotes(impl) < calculateQuorumThreshold() + 1) {
467+
return isReached = false;
468+
}
469+
if (lockedSupply() < Parameters.MIN_LOCKED_SUPPLY) {
470+
return isReached = false;
471+
}
472+
isReached = true;
482473
}
483474

484475
/// @notice Checks if the delay period for a scheduled upgrade has ended and reverts with errors if not.

‎src/interfaces/IXanV1.sol‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ interface IXanV1 {
6868
function cancelVoterBodyUpgrade() external;
6969

7070
/// @notice Schedules the upgrade to a new implementation. This is only callable by the council.
71-
/// @param proposedImpl The implementation proposed by the council.
72-
function scheduleCouncilUpgrade(address proposedImpl) external;
71+
/// @param impl The implementation proposed by the council.
72+
function scheduleCouncilUpgrade(address impl) external;
7373

7474
/// @notice Cancels the upgrade proposed by the governance council.
7575
/// This is only callable by the council.
@@ -123,9 +123,11 @@ interface IXanV1 {
123123
/// @return current The current implementation.
124124
function implementation() external view returns (address current);
125125

126-
/// @notice Returns the proposed implementation with the respective rank.
127-
/// @return rankedImplementation The proposed implementation with the respective rank.
128-
function proposedImplementationByRank(uint48 rank) external view returns (address rankedImplementation);
126+
/// @notice Returns the proposed implementation with the respective rank or an error if no implementation with this
127+
/// rank has been proposed yet.
128+
/// @param rank The rank to return the implementation for.
129+
/// @return impl The proposed implementation with the respective rank.
130+
function proposedImplementationByRank(uint48 rank) external view returns (address impl);
129131

130132
/// @notice Returns the address of the governance council.
131133
/// @return council The governance council address.

‎src/libs/Voting.sol‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,17 @@ library Voting {
113113
data.ballots[implA].rank = rankB;
114114
data.ballots[implB].rank = rankA;
115115
}
116+
117+
/// @notice Returns the implementation with the respective rank or `address(0)` if the rank does not exist.
118+
/// @param rank The rank to return the implementation for.
119+
/// @return impl The proposed implementation with the respective rank or `address(0)` if the rank does not exist.
120+
function implementationByRank(Data storage data, uint48 rank) internal view returns (address impl) {
121+
uint48 implCount = data.implCount;
122+
123+
if (implCount == 0 || rank > implCount - 1) {
124+
return impl = address(0);
125+
}
126+
127+
impl = data.ranking[rank];
128+
}
116129
}

‎test/XanV1.council.t.sol‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ contract XanV1CouncilTest is Test {
4848

4949
// Attempt to schedule an council upgrade.
5050
vm.prank(_COUNCIL);
51-
vm.expectRevert(abi.encodeWithSelector(XanV1.QuorumReached.selector, _NEW_IMPL), address(_xanProxy));
51+
vm.expectRevert(
52+
abi.encodeWithSelector(XanV1.QuorumAndMinLockedSupplyReached.selector, _NEW_IMPL), address(_xanProxy)
53+
);
5254
_xanProxy.scheduleCouncilUpgrade(_OTHER_NEW_IMPL);
5355
}
5456

@@ -64,16 +66,20 @@ contract XanV1CouncilTest is Test {
6466

6567
// Attempt to schedule an council upgrade.
6668
vm.prank(_COUNCIL);
67-
vm.expectRevert(abi.encodeWithSelector(XanV1.QuorumReached.selector, _NEW_IMPL), address(_xanProxy));
69+
vm.expectRevert(
70+
abi.encodeWithSelector(XanV1.QuorumAndMinLockedSupplyReached.selector, _NEW_IMPL), address(_xanProxy)
71+
);
6872
_xanProxy.scheduleCouncilUpgrade(_NEW_IMPL);
6973
}
7074

7175
function test_scheduleCouncilUpgrade_reverts_if_an_council_upgrade_has_been_proposed_already() public {
7276
vm.startPrank(_COUNCIL);
77+
78+
uint48 endTime = Time.timestamp() + Parameters.DELAY_DURATION;
7379
_xanProxy.scheduleCouncilUpgrade(_NEW_IMPL);
7480

7581
vm.expectRevert(
76-
abi.encodeWithSelector(XanV1.ImplementationAlreadyProposed.selector, _NEW_IMPL), address(_xanProxy)
82+
abi.encodeWithSelector(XanV1.UpgradeAlreadyScheduled.selector, _NEW_IMPL, endTime), address(_xanProxy)
7783
);
7884
_xanProxy.scheduleCouncilUpgrade(_NEW_IMPL);
7985
}
@@ -129,8 +135,20 @@ contract XanV1CouncilTest is Test {
129135
vm.prank(_COUNCIL);
130136
_xanProxy.scheduleCouncilUpgrade(_NEW_IMPL);
131137

138+
// Vote for another implementation but without meeting the minimal locked supply
139+
vm.startPrank(_defaultSender);
140+
// Lock first half.
141+
_xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY - 1);
142+
_xanProxy.castVote(_OTHER_NEW_IMPL);
143+
vm.stopPrank();
144+
132145
vm.prank(_defaultSender);
133-
vm.expectRevert(abi.encodeWithSelector(XanV1.QuorumNowhereReached.selector), address(_xanProxy));
146+
vm.expectRevert(
147+
abi.encodeWithSelector(
148+
XanV1.QuorumOrMinLockedSupplyNotReached.selector, _xanProxy.proposedImplementationByRank(0)
149+
),
150+
address(_xanProxy)
151+
);
134152
_xanProxy.vetoCouncilUpgrade();
135153
}
136154

‎test/XanV1.upgrade.t.sol‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,10 @@ contract XanV1UpgradeTest is Test {
106106
vm.prank(_defaultSender);
107107
_xanProxy.revokeVote(_voterProposedImpl);
108108

109-
vm.expectRevert(abi.encodeWithSelector(XanV1.QuorumNotReached.selector, _voterProposedImpl), address(_xanProxy));
109+
vm.expectRevert(
110+
abi.encodeWithSelector(XanV1.QuorumOrMinLockedSupplyNotReached.selector, _voterProposedImpl),
111+
address(_xanProxy)
112+
);
110113
_xanProxy.upgradeToAndCall({newImplementation: _voterProposedImpl, data: ""});
111114
}
112115

‎test/XanV1.voting.t.sol‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {Upgrades, UnsafeUpgrades} from "@openzeppelin/foundry-upgrades/Upgrades.
88
import {Test} from "forge-std/Test.sol";
99

1010
import {Parameters} from "../src/libs/Parameters.sol";
11+
import {Voting} from "../src/libs/Voting.sol";
1112
import {IXanV1, XanV1} from "../src/XanV1.sol";
1213

1314
contract XanV1VotingTest is Test {
@@ -199,8 +200,15 @@ contract XanV1VotingTest is Test {
199200
vm.startPrank(_defaultSender);
200201
// Lock first half.
201202
_xanProxy.lock(Parameters.MIN_LOCKED_SUPPLY - 1);
203+
_xanProxy.castVote(_NEW_IMPL);
204+
vm.stopPrank();
202205

203-
vm.expectRevert(abi.encodeWithSelector(XanV1.MinLockedSupplyNotReached.selector), address(_xanProxy));
206+
vm.expectRevert(
207+
abi.encodeWithSelector(
208+
XanV1.QuorumOrMinLockedSupplyNotReached.selector, _xanProxy.proposedImplementationByRank(0)
209+
),
210+
address(_xanProxy)
211+
);
204212
_xanProxy.scheduleVoterBodyUpgrade();
205213
}
206214

@@ -218,7 +226,9 @@ contract XanV1VotingTest is Test {
218226
_xanProxy.lock(quorumThreshold);
219227
vm.stopPrank();
220228

221-
vm.expectRevert(abi.encodeWithSelector(XanV1.QuorumNotReached.selector, _NEW_IMPL), address(_xanProxy));
229+
vm.expectRevert(
230+
abi.encodeWithSelector(XanV1.QuorumOrMinLockedSupplyNotReached.selector, _NEW_IMPL), address(_xanProxy)
231+
);
222232
_xanProxy.scheduleVoterBodyUpgrade();
223233
}
224234

0 commit comments

Comments
 (0)