diff --git a/contracts/Escrow.sol b/contracts/Escrow.sol index 1a24919e..aba6720c 100644 --- a/contracts/Escrow.sol +++ b/contracts/Escrow.sol @@ -35,11 +35,11 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { error EmptyUnstETHIds(); error UnclaimedBatches(); - error UnexpectedUnstETHId(); error UnfinalizedUnstETHIds(); error NonProxyCallsForbidden(); error BatchesQueueIsNotClosed(); error InvalidBatchSize(uint256 size); + error InvalidFromUnstETHId(uint256 unstETHId); error CallerIsNotDualGovernance(address caller); error InvalidHintsLength(uint256 actual, uint256 expected); error InvalidETHSender(address actual, address expected); @@ -489,15 +489,15 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { revert BatchesQueueIsNotClosed(); } - /// @dev This check is primarily required when only unstETH NFTs are locked in the Escrow - /// and there are no WithdrawalsBatches. In this scenario, the RageQuitExtensionPeriod can only begin - /// when the last locked unstETH id is finalized in the WithdrawalQueue. - /// When the WithdrawalsBatchesQueue is not empty, this invariant is maintained by the following: + /// @dev This check is required when only unstETH NFTs are locked in the Escrow and there are no WithdrawalsBatches. + /// In this scenario, the RageQuitExtensionPeriod can only begin when the last locked unstETH id is finalized + /// in the WithdrawalQueue. When the WithdrawalsBatchesQueue is not empty, this invariant is maintained by + /// the following: /// - Any locked unstETH during the VetoSignalling phase has an id less than any unstETH NFT created /// during the request for withdrawal batches. /// - Claiming the withdrawal batches requires the finalization of the unstETH with the given id. /// - The finalization of unstETH NFTs occurs in FIFO order. - if (_batchesQueue.getLastClaimedOrBoundaryUnstETHId() > WITHDRAWAL_QUEUE.getLastFinalizedRequestId()) { + if (_batchesQueue.getBoundaryUnstETHId() > WITHDRAWAL_QUEUE.getLastFinalizedRequestId()) { revert UnfinalizedUnstETHIds(); } @@ -629,7 +629,7 @@ contract Escrow is ISignallingEscrow, IRageQuitEscrow { uint256[] memory hints ) internal { if (fromUnstETHId != unstETHIds[0]) { - revert UnexpectedUnstETHId(); + revert InvalidFromUnstETHId(fromUnstETHId); } if (hints.length != unstETHIds.length) { diff --git a/contracts/interfaces/ISignallingEscrow.sol b/contracts/interfaces/ISignallingEscrow.sol index de91a3a6..fcdfcc43 100644 --- a/contracts/interfaces/ISignallingEscrow.sol +++ b/contracts/interfaces/ISignallingEscrow.sol @@ -9,7 +9,6 @@ import {SharesValue} from "../types/SharesValue.sol"; import {UnstETHRecordStatus} from "../libraries/AssetsAccounting.sol"; import {IEscrowBase} from "./IEscrowBase.sol"; -import {IRageQuitEscrow} from "./IRageQuitEscrow.sol"; interface ISignallingEscrow is IEscrowBase { struct VetoerDetails { diff --git a/contracts/libraries/DualGovernanceStateMachine.sol b/contracts/libraries/DualGovernanceStateMachine.sol index 83a6d73b..d384fc22 100644 --- a/contracts/libraries/DualGovernanceStateMachine.sol +++ b/contracts/libraries/DualGovernanceStateMachine.sol @@ -17,7 +17,7 @@ import {DualGovernanceConfig} from "./DualGovernanceConfig.sol"; import {DualGovernanceStateTransitions} from "./DualGovernanceStateTransitions.sol"; /// @notice Enum describing the state of the Dual Governance State Machine -/// @param Unset The initial (uninitialized) state of the Dual Governance State Machine. The state machine cannot +/// @param NotInitialized The initial (uninitialized) state of the Dual Governance State Machine. The state machine cannot /// operate in this state and must be initialized before use. /// @param Normal The default state where the system is expected to remain most of the time. In this state, proposals /// can be both submitted and scheduled for execution. @@ -32,7 +32,7 @@ import {DualGovernanceStateTransitions} from "./DualGovernanceStateTransitions.s /// is triggered when the Second Seal Threshold is reached. During this state, the scheduling of proposals for /// execution is forbidden, but new proposals can still be submitted. enum State { - Unset, + NotInitialized, Normal, VetoSignalling, VetoSignallingDeactivation, @@ -118,7 +118,7 @@ library DualGovernanceStateMachine { IDualGovernanceConfigProvider configProvider, IEscrowBase escrowMasterCopy ) internal { - if (self.state != State.Unset) { + if (self.state != State.NotInitialized) { revert AlreadyInitialized(); } @@ -130,7 +130,7 @@ library DualGovernanceStateMachine { DualGovernanceConfig.Context memory config = configProvider.getDualGovernanceConfig(); _deployNewSignallingEscrow(self, escrowMasterCopy, config.minAssetsLockDuration); - emit DualGovernanceStateChanged(State.Unset, State.Normal, self); + emit DualGovernanceStateChanged(State.NotInitialized, State.Normal, self); } /// @notice Executes a state transition for the Dual Governance State Machine, if applicable. diff --git a/contracts/libraries/EmergencyProtection.sol b/contracts/libraries/EmergencyProtection.sol index b3df9833..4543a79a 100644 --- a/contracts/libraries/EmergencyProtection.sol +++ b/contracts/libraries/EmergencyProtection.sol @@ -22,7 +22,7 @@ library EmergencyProtection { error InvalidEmergencyExecutionCommittee(address committee); error InvalidEmergencyModeDuration(Duration value); error InvalidEmergencyProtectionEndDate(Timestamp value); - error UnexpectedEmergencyModeState(bool value); + error UnexpectedEmergencyModeState(bool state); // --- // Events @@ -197,7 +197,7 @@ library EmergencyProtection { /// @param isActive The expected value of the emergency mode. function checkEmergencyMode(Context storage self, bool isActive) internal view { if (isEmergencyModeActive(self) != isActive) { - revert UnexpectedEmergencyModeState(isActive); + revert UnexpectedEmergencyModeState(!isActive); } } diff --git a/contracts/libraries/EscrowState.sol b/contracts/libraries/EscrowState.sol index 9fb30ff3..dbfbee59 100644 --- a/contracts/libraries/EscrowState.sol +++ b/contracts/libraries/EscrowState.sol @@ -26,7 +26,7 @@ library EscrowState { // --- error ClaimingIsFinished(); - error UnexpectedState(State value); + error UnexpectedEscrowState(State state); error EthWithdrawalsDelayNotPassed(); error RageQuitExtensionPeriodNotStarted(); error InvalidMinAssetsLockDuration(Duration newMinAssetsLockDuration); @@ -184,7 +184,7 @@ library EscrowState { /// @param state The expected state. function _checkState(Context storage self, State state) private view { if (self.state != state) { - revert UnexpectedState(state); + revert UnexpectedEscrowState(self.state); } } diff --git a/contracts/libraries/ExecutableProposals.sol b/contracts/libraries/ExecutableProposals.sol index 070af315..295b8e6e 100644 --- a/contracts/libraries/ExecutableProposals.sol +++ b/contracts/libraries/ExecutableProposals.sol @@ -79,9 +79,7 @@ library ExecutableProposals { // --- error EmptyCalls(); - error ProposalNotFound(uint256 proposalId); - error ProposalNotScheduled(uint256 proposalId); - error ProposalNotSubmitted(uint256 proposalId); + error UnexpectedProposalStatus(uint256 proposalId, Status status); error AfterSubmitDelayNotPassed(uint256 proposalId); error AfterScheduleDelayNotPassed(uint256 proposalId); @@ -141,19 +139,21 @@ library ExecutableProposals { /// @param afterSubmitDelay The required delay duration after submission before the proposal can be scheduled. /// function schedule(Context storage self, uint256 proposalId, Duration afterSubmitDelay) internal { - ProposalData memory proposalState = self.proposals[proposalId].data; + ProposalData memory proposalData = self.proposals[proposalId].data; + + _checkProposalNotCancelled(self, proposalId, proposalData); - if (!_isProposalSubmitted(self, proposalId, proposalState)) { - revert ProposalNotSubmitted(proposalId); + if (proposalData.status != Status.Submitted) { + revert UnexpectedProposalStatus(proposalId, proposalData.status); } - if (afterSubmitDelay.addTo(proposalState.submittedAt) > Timestamps.now()) { + if (afterSubmitDelay.addTo(proposalData.submittedAt) > Timestamps.now()) { revert AfterSubmitDelayNotPassed(proposalId); } - proposalState.status = Status.Scheduled; - proposalState.scheduledAt = Timestamps.now(); - self.proposals[proposalId].data = proposalState; + proposalData.status = Status.Scheduled; + proposalData.scheduledAt = Timestamps.now(); + self.proposals[proposalId].data = proposalData; emit ProposalScheduled(proposalId); } @@ -166,8 +166,10 @@ library ExecutableProposals { function execute(Context storage self, uint256 proposalId, Duration afterScheduleDelay) internal { Proposal memory proposal = self.proposals[proposalId]; - if (!_isProposalScheduled(self, proposalId, proposal.data)) { - revert ProposalNotScheduled(proposalId); + _checkProposalNotCancelled(self, proposalId, proposal.data); + + if (proposal.data.status != Status.Scheduled) { + revert UnexpectedProposalStatus(proposalId, proposal.data.status); } if (afterScheduleDelay.addTo(proposal.data.scheduledAt) > Timestamps.now()) { @@ -192,21 +194,6 @@ library ExecutableProposals { // Getters // --- - /// @notice Determines whether a proposal is eligible for execution based on its status and delay requirements. - /// @param self The context of the Executable Proposal library. - /// @param proposalId The id of the proposal to check for execution eligibility. - /// @param afterScheduleDelay The required delay duration after scheduling before the proposal can be executed. - /// @return bool `true` if the proposal is eligible for execution, otherwise `false`. - function canExecute( - Context storage self, - uint256 proposalId, - Duration afterScheduleDelay - ) internal view returns (bool) { - ProposalData memory proposalState = self.proposals[proposalId].data; - return _isProposalScheduled(self, proposalId, proposalState) - && Timestamps.now() >= afterScheduleDelay.addTo(proposalState.scheduledAt); - } - /// @notice Determines whether a proposal is eligible to be scheduled based on its status and required delay. /// @param self The context of the Executable Proposal library. /// @param proposalId The id of the proposal to check for scheduling eligibility. @@ -217,9 +204,24 @@ library ExecutableProposals { uint256 proposalId, Duration afterSubmitDelay ) internal view returns (bool) { - ProposalData memory proposalState = self.proposals[proposalId].data; - return _isProposalSubmitted(self, proposalId, proposalState) - && Timestamps.now() >= afterSubmitDelay.addTo(proposalState.submittedAt); + ProposalData memory proposalData = self.proposals[proposalId].data; + return proposalId > self.lastCancelledProposalId && proposalData.status == Status.Submitted + && Timestamps.now() >= afterSubmitDelay.addTo(proposalData.submittedAt); + } + + /// @notice Determines whether a proposal is eligible for execution based on its status and delay requirements. + /// @param self The context of the Executable Proposal library. + /// @param proposalId The id of the proposal to check for execution eligibility. + /// @param afterScheduleDelay The required delay duration after scheduling before the proposal can be executed. + /// @return bool `true` if the proposal is eligible for execution, otherwise `false`. + function canExecute( + Context storage self, + uint256 proposalId, + Duration afterScheduleDelay + ) internal view returns (bool) { + ProposalData memory proposalData = self.proposals[proposalId].data; + return proposalId > self.lastCancelledProposalId && proposalData.status == Status.Scheduled + && Timestamps.now() >= afterScheduleDelay.addTo(proposalData.scheduledAt); } /// @notice Returns the total count of submitted proposals. @@ -268,24 +270,18 @@ library ExecutableProposals { function _checkProposalExists(uint256 proposalId, ProposalData memory proposalData) private pure { if (proposalData.status == Status.NotExist) { - revert ProposalNotFound(proposalId); + revert UnexpectedProposalStatus(proposalId, Status.NotExist); } } - function _isProposalSubmitted( + function _checkProposalNotCancelled( Context storage self, uint256 proposalId, ProposalData memory proposalData - ) private view returns (bool) { - return proposalId > self.lastCancelledProposalId && proposalData.status == Status.Submitted; - } - - function _isProposalScheduled( - Context storage self, - uint256 proposalId, - ProposalData memory proposalData - ) private view returns (bool) { - return proposalId > self.lastCancelledProposalId && proposalData.status == Status.Scheduled; + ) private view { + if (_isProposalCancelled(self, proposalId, proposalData)) { + revert UnexpectedProposalStatus(proposalId, Status.Cancelled); + } } function _isProposalCancelled( diff --git a/contracts/libraries/WithdrawalsBatchesQueue.sol b/contracts/libraries/WithdrawalsBatchesQueue.sol index 6adf5b5b..a84339d1 100644 --- a/contracts/libraries/WithdrawalsBatchesQueue.sol +++ b/contracts/libraries/WithdrawalsBatchesQueue.sol @@ -5,11 +5,11 @@ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; /// @notice The state of the WithdrawalBatchesQueue. -/// @param Absent The initial (uninitialized) state of the WithdrawalBatchesQueue. +/// @param NotInitialized The initial (uninitialized) state of the WithdrawalBatchesQueue. /// @param Opened In this state, the WithdrawalBatchesQueue allows the addition of new batches of unstETH ids. /// @param Closed The terminal state of the queue where adding new batches is no longer permitted. enum State { - Absent, + NotInitialized, Opened, Closed } @@ -23,9 +23,7 @@ library WithdrawalsBatchesQueue { error EmptyBatch(); error InvalidUnstETHIdsSequence(); - error WithdrawalsBatchesQueueIsInAbsentState(); - error WithdrawalsBatchesQueueIsNotInOpenedState(); - error WithdrawalsBatchesQueueIsNotInAbsentState(); + error UnexpectedWithdrawalsBatchesQueueState(State state); // --- // Events @@ -91,9 +89,7 @@ library WithdrawalsBatchesQueue { /// @param boundaryUnstETHId The id of the unstETH NFT which is used as the boundary value for the withdrawal queue. /// `boundaryUnstETHId` value is used as a lower bound for the adding unstETH ids. function open(Context storage self, uint256 boundaryUnstETHId) internal { - if (self.info.state != State.Absent) { - revert WithdrawalsBatchesQueueIsNotInAbsentState(); - } + _checkState(self, State.NotInitialized); self.info.state = State.Opened; @@ -108,7 +104,7 @@ library WithdrawalsBatchesQueue { /// @param self The context of the Withdrawals Batches Queue library. /// @param unstETHIds An array of sequential unstETH ids to be added to the queue. function addUnstETHIds(Context storage self, uint256[] memory unstETHIds) internal { - _checkInOpenedState(self); + _checkState(self, State.Opened); uint256 unstETHIdsCount = unstETHIds.length; @@ -163,7 +159,7 @@ library WithdrawalsBatchesQueue { /// @notice Closes the WithdrawalsBatchesQueue, preventing further batch additions. /// @param self The context of the Withdrawals Batches Queue library. function close(Context storage self) internal { - _checkInOpenedState(self); + _checkState(self, State.Opened); self.info.state = State.Closed; emit WithdrawalsBatchesQueueClosed(); } @@ -220,13 +216,13 @@ library WithdrawalsBatchesQueue { return self.info.totalUnstETHIdsCount - self.info.totalUnstETHIdsClaimed; } - /// @notice Returns the id of the last claimed unstETH. When the queue is empty, returns 0. + /// @notice Returns the id of the boundary unstETH. + /// @dev Reverts with an index OOB error if called when the `WithdrawalsBatchesQueue` is in the + /// `NotInitialized` state. /// @param self The context of the Withdrawals Batches Queue library. - /// @return lastClaimedUnstETHId The id of the lastClaimedUnstETHId or 0 when the queue is empty - function getLastClaimedOrBoundaryUnstETHId(Context storage self) internal view returns (uint256) { - _checkNotInAbsentState(self); - QueueInfo memory info = self.info; - return self.batches[info.lastClaimedBatchIndex].firstUnstETHId + info.lastClaimedUnstETHIdIndex; + /// @return boundaryUnstETHId The id of the boundary unstETH. + function getBoundaryUnstETHId(Context storage self) internal view returns (uint256) { + return self.batches[0].firstUnstETHId; } /// @notice Returns if all unstETH ids in the queue have been claimed. @@ -277,15 +273,9 @@ library WithdrawalsBatchesQueue { info.totalUnstETHIdsClaimed += SafeCast.toUint64(unstETHIdsCount); } - function _checkNotInAbsentState(Context storage self) private view { - if (self.info.state == State.Absent) { - revert WithdrawalsBatchesQueueIsInAbsentState(); - } - } - - function _checkInOpenedState(Context storage self) private view { - if (self.info.state != State.Opened) { - revert WithdrawalsBatchesQueueIsNotInOpenedState(); + function _checkState(Context storage self, State expectedState) private view { + if (self.info.state != expectedState) { + revert UnexpectedWithdrawalsBatchesQueueState(self.info.state); } } } diff --git a/docs/specification.md b/docs/specification.md index d62816f2..9d6ecb76 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -232,7 +232,7 @@ This contract is a singleton, meaning that any DG deployment includes exactly on ```solidity enum State { - Unset, // Indicates an uninitialized state during the contract creation + NotInitialized, // Indicates an uninitialized state during the contract creation Normal, VetoSignalling, VetoSignallingDeactivation, diff --git a/test/scenario/dg-update-tokens-rotation.t.sol b/test/scenario/dg-update-tokens-rotation.t.sol index 88d22b60..7a59727b 100644 --- a/test/scenario/dg-update-tokens-rotation.t.sol +++ b/test/scenario/dg-update-tokens-rotation.t.sol @@ -197,7 +197,9 @@ contract DualGovernanceUpdateTokensRotation is ScenarioTestBlueprint { _step("7. After the update malicious proposal is cancelled and can't be executed via new DualGovernance"); { vm.expectRevert( - abi.encodeWithSelector(ExecutableProposals.ProposalNotSubmitted.selector, maliciousProposalId) + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, maliciousProposalId, ProposalStatus.Cancelled + ) ); newDualGovernanceInstance.scheduleProposal(maliciousProposalId); diff --git a/test/scenario/escrow.t.sol b/test/scenario/escrow.t.sol index 26604958..8470f52c 100644 --- a/test/scenario/escrow.t.sol +++ b/test/scenario/escrow.t.sol @@ -488,22 +488,22 @@ contract EscrowHappyPath is ScenarioTestBlueprint { // After the Escrow enters RageQuitEscrow state, lock/unlock of tokens is forbidden // --- - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalLockStETH(_VETOER_1, 1 ether); - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalLockWstETH(_VETOER_1, 1 ether); - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalLockUnstETH(_VETOER_1, notLockedWithdrawalNfts); - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalUnlockStETH(_VETOER_1); - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalUnlockWstETH(_VETOER_1); - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalUnlockUnstETH(_VETOER_1, lockedWithdrawalNfts); } @@ -547,7 +547,7 @@ contract EscrowHappyPath is ScenarioTestBlueprint { vm.revertTo(snapshotId); // The attempt to unlock funds from Escrow will fail - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); this.externalUnlockStETH(_VETOER_1); } diff --git a/test/scenario/happy-path-plan-b.t.sol b/test/scenario/happy-path-plan-b.t.sol index 6681778d..b387c9c6 100644 --- a/test/scenario/happy-path-plan-b.t.sol +++ b/test/scenario/happy-path-plan-b.t.sol @@ -14,8 +14,8 @@ import { } from "../utils/scenario-test-blueprint.sol"; import {DualGovernance} from "contracts/DualGovernance.sol"; -import {ExecutableProposals} from "contracts/libraries/ExecutableProposals.sol"; import {EmergencyProtection} from "contracts/libraries/EmergencyProtection.sol"; +import {ExecutableProposals, Status as ProposalStatus} from "contracts/libraries/ExecutableProposals.sol"; contract PlanBSetup is ScenarioTestBlueprint { function setUp() external { @@ -97,7 +97,7 @@ contract PlanBSetup is ScenarioTestBlueprint { // but the call still not executable _assertCanExecute(maliciousProposalId, false); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(maliciousProposalId); } @@ -325,7 +325,7 @@ contract PlanBSetup is ScenarioTestBlueprint { _wait(_timelock.getAfterScheduleDelay().plusSeconds(1)); _assertCanExecute(maliciousProposalId, false); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(maliciousProposalId); } @@ -350,7 +350,7 @@ contract PlanBSetup is ScenarioTestBlueprint { _wait(_timelock.getAfterScheduleDelay().plusSeconds(1)); _assertCanExecute(anotherMaliciousProposalId, false); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(anotherMaliciousProposalId); } @@ -359,10 +359,10 @@ contract PlanBSetup is ScenarioTestBlueprint { _wait(_EMERGENCY_MODE_DURATION.dividedBy(2)); assertTrue(emergencyState.emergencyModeEndsAfter < Timestamps.now()); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(maliciousProposalId); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(anotherMaliciousProposalId); } @@ -380,12 +380,18 @@ contract PlanBSetup is ScenarioTestBlueprint { _assertProposalCancelled(anotherMaliciousProposalId); vm.expectRevert( - abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, maliciousProposalId) + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, maliciousProposalId, ProposalStatus.Cancelled + ) ); _executeProposal(maliciousProposalId); vm.expectRevert( - abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, anotherMaliciousProposalId) + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, + anotherMaliciousProposalId, + ProposalStatus.Cancelled + ) ); _executeProposal(anotherMaliciousProposalId); } diff --git a/test/scenario/timelocked-governance.t.sol b/test/scenario/timelocked-governance.t.sol index d6be5be5..c822e75f 100644 --- a/test/scenario/timelocked-governance.t.sol +++ b/test/scenario/timelocked-governance.t.sol @@ -99,7 +99,7 @@ contract TimelockedGovernanceScenario is ScenarioTestBlueprint { _assertCanExecute(maliciousProposalId, false); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(maliciousProposalId); } @@ -174,7 +174,7 @@ contract TimelockedGovernanceScenario is ScenarioTestBlueprint { _assertCanExecute(maliciousProposalId, false); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _executeProposal(maliciousProposalId); } diff --git a/test/unit/EmergencyProtectedTimelock.t.sol b/test/unit/EmergencyProtectedTimelock.t.sol index b0881965..bc00a00c 100644 --- a/test/unit/EmergencyProtectedTimelock.t.sol +++ b/test/unit/EmergencyProtectedTimelock.t.sol @@ -272,7 +272,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { _activateEmergencyMode(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [false])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _timelock.execute(1); ITimelock.ProposalDetails memory proposal = _timelock.getProposalDetails(1); @@ -561,7 +561,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { assertEq(_isEmergencyStateActivated(), true); vm.prank(_emergencyActivator); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [false])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); _timelock.activateEmergencyMode(); assertEq(_isEmergencyStateActivated(), true); @@ -606,7 +606,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { assertEq(_timelock.isEmergencyModeActive(), false); vm.prank(_emergencyActivator); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [true])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); _timelock.emergencyExecute(1); } @@ -683,11 +683,11 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { vm.assume(stranger != _adminExecutor); vm.prank(stranger); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [true])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); _timelock.deactivateEmergencyMode(); vm.prank(_adminExecutor); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [true])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); _timelock.deactivateEmergencyMode(); } @@ -764,7 +764,7 @@ contract EmergencyProtectedTimelockUnitTests is UnitTest { address emergencyActivationCommitteeBefore = _timelock.getEmergencyActivationCommittee(); address emergencyExecutionCommitteeBefore = _timelock.getEmergencyExecutionCommittee(); - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, [true])); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); vm.prank(_emergencyEnactor); _timelock.emergencyReset(); diff --git a/test/unit/Escrow.t.sol b/test/unit/Escrow.t.sol index 1572bc39..e4179f97 100644 --- a/test/unit/Escrow.t.sol +++ b/test/unit/Escrow.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.26; -import {stdError} from "forge-std/StdError.sol"; import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; @@ -198,7 +197,9 @@ contract EscrowUnitTests is UnitTest { function test_lockStETH_RevertOn_UnexpectedEscrowState() external { _transitToRageQuit(); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); vm.prank(_vetoer); _escrow.lockStETH(1 ether); @@ -252,7 +253,9 @@ contract EscrowUnitTests is UnitTest { _transitToRageQuit(); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); vm.prank(_vetoer); _escrow.unlockStETH(); } @@ -317,7 +320,9 @@ contract EscrowUnitTests is UnitTest { function test_lockWstETH_RevertOn_UnexpectedEscrowState() external { _transitToRageQuit(); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); vm.prank(_vetoer); _escrow.lockWstETH(1 ether); } @@ -376,7 +381,9 @@ contract EscrowUnitTests is UnitTest { _transitToRageQuit(); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); vm.prank(_vetoer); _escrow.unlockWstETH(); } @@ -459,7 +466,9 @@ contract EscrowUnitTests is UnitTest { _transitToRageQuit(); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); _escrow.lockUnstETH(unstethIds); } @@ -532,7 +541,9 @@ contract EscrowUnitTests is UnitTest { _transitToRageQuit(); uint256[] memory unstethIds = new uint256[](1); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); vm.prank(_vetoer); _escrow.unlockUnstETH(unstethIds); } @@ -569,7 +580,9 @@ contract EscrowUnitTests is UnitTest { uint256[] memory unstethIds = new uint256[](0); uint256[] memory hints = new uint256[](0); - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.SignallingEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.RageQuitEscrow) + ); _escrow.markUnstETHFinalized(unstethIds, hints); } @@ -630,7 +643,9 @@ contract EscrowUnitTests is UnitTest { } function test_requestNextWithdrawalsBatch_RevertOn_UnexpectedEscrowState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.requestNextWithdrawalsBatch(1); } @@ -711,8 +726,10 @@ contract EscrowUnitTests is UnitTest { assertEq(state.unstETHLockedShares.toUint256(), 0); } - function test_claimNextWithdrawalsBatch_2_RevertOn_UnexpectedState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, 2)); + function test_claimNextWithdrawalsBatch_2_RevertOn_UnexpectedEscrowState() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.claimNextWithdrawalsBatch(1, new uint256[](1)); } @@ -733,7 +750,7 @@ contract EscrowUnitTests is UnitTest { _escrow.claimNextWithdrawalsBatch(1, new uint256[](1)); } - function test_claimNextWithdrawalsBatch_2_RevertOn_UnexpectedUnstETHId() external { + function test_claimNextWithdrawalsBatch_2_RevertOn_InvalidFromUnstETHId() external { uint256[] memory unstEthIds = _getUnstEthIdsFromWQ(); _vetoerLockedStEth(stethAmount); @@ -746,7 +763,7 @@ contract EscrowUnitTests is UnitTest { _withdrawalQueue.setClaimableAmount(stethAmount); vm.deal(address(_withdrawalQueue), stethAmount); - vm.expectRevert(Escrow.UnexpectedUnstETHId.selector); + vm.expectRevert(abi.encodeWithSelector(Escrow.InvalidFromUnstETHId.selector, unstEthIds[0] + 10)); _escrow.claimNextWithdrawalsBatch(unstEthIds[0] + 10, new uint256[](1)); } @@ -810,8 +827,10 @@ contract EscrowUnitTests is UnitTest { assertEq(vetoerState.unstETHLockedShares.toUint256(), 0); } - function test_claimNextWithdrawalsBatch_1_RevertOn_UnexpectedState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, 2)); + function test_claimNextWithdrawalsBatch_1_RevertOn_UnexpectedEscrowState() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.claimNextWithdrawalsBatch(1); } @@ -926,12 +945,13 @@ contract EscrowUnitTests is UnitTest { } function test_claimUnstETH_RevertOn_UnexpectedEscrowState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.claimUnstETH(new uint256[](1), new uint256[](1)); } function test_claimUnstETH_RevertOn_InvalidRequestId() external { - bytes memory wqInvalidRequestIdError = abi.encode("WithdrawalQueue.InvalidRequestId"); uint256[] memory unstETHIds = new uint256[](1); unstETHIds[0] = _withdrawalQueue.REVERT_ON_ID(); uint256[] memory hints = new uint256[](1); @@ -943,7 +963,6 @@ contract EscrowUnitTests is UnitTest { } function test_claimUnstETH_RevertOn_ArraysLengthMismatch() external { - bytes memory wqArraysLengthMismatchError = abi.encode("WithdrawalQueue.ArraysLengthMismatch"); uint256[] memory unstETHIds = new uint256[](2); uint256[] memory hints = new uint256[](1); uint256[] memory responses = new uint256[](1); @@ -1045,7 +1064,9 @@ contract EscrowUnitTests is UnitTest { } function test_withdrawETH_RevertOn_UnexpectedEscrowState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.withdrawETH(); } @@ -1167,7 +1188,9 @@ contract EscrowUnitTests is UnitTest { } function test_withdrawETH_2_RevertOn_UnexpectedEscrowState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.withdrawETH(new uint256[](1)); } @@ -1383,19 +1406,25 @@ contract EscrowUnitTests is UnitTest { } function test_getNextWithdrawalBatch_RevertOn_RageQuit_IsNotStarted() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.getNextWithdrawalBatch(100); } - function test_getNextWithdrawalBatch_RevertOn_UnexpectedState_Signaling() external { + function test_getNextWithdrawalBatch_RevertOn_UnexpectedEscrowState_Signaling() external { uint256 batchLimit = 10; - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.getNextWithdrawalBatch(batchLimit); } - function test_getNextWithdrawalBatch_RevertOn_UnexpectedState_NotInitialized() external { + function test_getNextWithdrawalBatch_RevertOn_UnexpectedEscrowState_NotInitialized() external { uint256 batchLimit = 10; - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.NotInitialized) + ); _masterCopy.getNextWithdrawalBatch(batchLimit); } @@ -1414,16 +1443,38 @@ contract EscrowUnitTests is UnitTest { assertTrue(_escrow.isWithdrawalsBatchesClosed()); } - function test_isWithdrawalsBatchesClosed_RevertOn_UnexpectedState_Signaling() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + function test_isWithdrawalsBatchesClosed_RevertOn_UnexpectedEscrowState_Signaling() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.isWithdrawalsBatchesClosed(); } - function test_isWithdrawalsBatchesClosed_RevertOn_UnexpectedState_NotInitialized() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + function test_isWithdrawalsBatchesClosed_RevertOn_UnexpectedEscrowState_NotInitialized() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.NotInitialized) + ); _masterCopy.isWithdrawalsBatchesClosed(); } + // --- + // getUnclaimedUnstETHIdsCount() + // --- + + function test_getUnclaimedUnstETHIdsCount_RevertOn_UnexpectedEscrowState_Signaling() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); + _escrow.getUnclaimedUnstETHIdsCount(); + } + + function test_getUnclaimedUnstETHIdsCount_RevertOn_UnexpectedEscrowState_NotInitialized() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.NotInitialized) + ); + _masterCopy.getUnclaimedUnstETHIdsCount(); + } + // --- // isRageQuitExtensionPeriodStarted() // --- @@ -1447,12 +1498,16 @@ contract EscrowUnitTests is UnitTest { // --- function test_getRageQuitExtensionPeriodStartedAt_RevertOn_NotInitializedState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.NotInitialized) + ); _masterCopy.getRageQuitEscrowDetails(); } function test_getRageQuitExtensionPeriodStartedAt_RevertOn_SignallingState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.getRageQuitEscrowDetails().rageQuitExtensionPeriodStartedAt; } @@ -1522,13 +1577,17 @@ contract EscrowUnitTests is UnitTest { // getRageQuitEscrowDetails() // --- - function test_getRageQuitEscrowDetails_RevertOn_UnexpectedState_Signaling() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + function test_getRageQuitEscrowDetails_RevertOn_UnexpectedEscrowState_Signaling() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.SignallingEscrow) + ); _escrow.getRageQuitEscrowDetails(); } - function test_getRageQuitEscrowDetails_RevertOn_UnexpectedState_NotInitialized() external { - vm.expectRevert(abi.encodeWithSelector(EscrowStateLib.UnexpectedState.selector, EscrowState.RageQuitEscrow)); + function test_getRageQuitEscrowDetails_RevertOn_UnexpectedEscrowState_NotInitialized() external { + vm.expectRevert( + abi.encodeWithSelector(EscrowStateLib.UnexpectedEscrowState.selector, EscrowState.NotInitialized) + ); _masterCopy.getRageQuitEscrowDetails(); } diff --git a/test/unit/libraries/DualGovernanceStateTransitions.t.sol b/test/unit/libraries/DualGovernanceStateTransitions.t.sol index 9c3abf47..d012b7ad 100644 --- a/test/unit/libraries/DualGovernanceStateTransitions.t.sol +++ b/test/unit/libraries/DualGovernanceStateTransitions.t.sol @@ -315,11 +315,11 @@ contract DualGovernanceStateTransitionsUnitTestSuite is UnitTest { } // --- - // Unset -> assert(false) + // NotInitialized -> assert(false) // --- - function test_getStateTransition_RevertOn_UnsetState() external { - _stateMachine.state = State.Unset; + function test_getStateTransition_RevertOn_NotInitializedState() external { + _stateMachine.state = State.NotInitialized; vm.expectRevert(stdError.assertionError); this.external__getStateTransition(); diff --git a/test/unit/libraries/EmergencyProtection.t.sol b/test/unit/libraries/EmergencyProtection.t.sol index 4fbb01ab..28c238c9 100644 --- a/test/unit/libraries/EmergencyProtection.t.sol +++ b/test/unit/libraries/EmergencyProtection.t.sol @@ -194,7 +194,7 @@ contract EmergencyProtectionTest is UnitTest { } function test_checkEmergencyMode_RevertOn_NotInEmergencyMode() external { - vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, true)); + vm.expectRevert(abi.encodeWithSelector(EmergencyProtection.UnexpectedEmergencyModeState.selector, false)); EmergencyProtection.checkEmergencyMode(ctx, true); } diff --git a/test/unit/libraries/EscrowState.t.sol b/test/unit/libraries/EscrowState.t.sol index c7685468..ecb91ee2 100644 --- a/test/unit/libraries/EscrowState.t.sol +++ b/test/unit/libraries/EscrowState.t.sol @@ -38,8 +38,7 @@ contract EscrowStateUnitTests is UnitTest { function testFuzz_initialize_RevertOn_InvalidState(Duration minAssetsLockDuration) external { _context.state = State.SignallingEscrow; - // TODO: not very informative, maybe need to change to `revert UnexpectedState(self.state);`: UnexpectedState(NotInitialized)[current implementation] => UnexpectedState(SignallingEscrow)[proposed] - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.NotInitialized)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.SignallingEscrow)); EscrowState.initialize(_context, minAssetsLockDuration); } @@ -75,7 +74,7 @@ contract EscrowStateUnitTests is UnitTest { ) external { _context.state = State.NotInitialized; - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.NotInitialized)); EscrowState.startRageQuit(_context, rageQuitExtensionPeriodDuration, rageQuitEthWithdrawalsDelay); } @@ -139,8 +138,11 @@ contract EscrowStateUnitTests is UnitTest { } function test_checkSignallingEscrow_RevertOn_InvalidState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.SignallingEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.NotInitialized)); + EscrowState.checkSignallingEscrow(_context); + _context.state = State.RageQuitEscrow; + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.RageQuitEscrow)); EscrowState.checkSignallingEscrow(_context); } @@ -154,8 +156,11 @@ contract EscrowStateUnitTests is UnitTest { } function test_checkRageQuitEscrow_RevertOn_InvalidState() external { - vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedState.selector, State.RageQuitEscrow)); + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.NotInitialized)); + EscrowState.checkRageQuitEscrow(_context); + _context.state = State.SignallingEscrow; + vm.expectRevert(abi.encodeWithSelector(EscrowState.UnexpectedEscrowState.selector, State.SignallingEscrow)); EscrowState.checkRageQuitEscrow(_context); } diff --git a/test/unit/libraries/ExecutableProposals.t.sol b/test/unit/libraries/ExecutableProposals.t.sol index d95c328a..6da61154 100644 --- a/test/unit/libraries/ExecutableProposals.t.sol +++ b/test/unit/libraries/ExecutableProposals.t.sol @@ -101,7 +101,11 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_cannot_schedule_unsubmitted_proposal(uint256 proposalId) external { vm.assume(proposalId > 0); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotSubmitted.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.NotExist + ) + ); _proposals.schedule(proposalId, Durations.ZERO); } @@ -110,7 +114,11 @@ contract ExecutableProposalsUnitTests is UnitTest { uint256 proposalId = 1; _proposals.schedule(proposalId, Durations.ZERO); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotSubmitted.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Scheduled + ) + ); _proposals.schedule(proposalId, Durations.ZERO); } @@ -133,7 +141,11 @@ contract ExecutableProposalsUnitTests is UnitTest { uint256 proposalId = _proposals.getProposalsCount(); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotSubmitted.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Cancelled + ) + ); _proposals.schedule(proposalId, Durations.ZERO); } @@ -171,7 +183,11 @@ contract ExecutableProposalsUnitTests is UnitTest { function testFuzz_cannot_execute_unsubmitted_proposal(uint256 proposalId) external { vm.assume(proposalId > 0); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.NotExist + ) + ); _proposals.execute(proposalId, Durations.ZERO); } @@ -179,7 +195,11 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.submit(proposer, address(_executor), _getMockTargetRegularStaffCalls(address(_targetMock)), ""); uint256 proposalId = _proposals.getProposalsCount(); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Submitted + ) + ); _proposals.execute(proposalId, Durations.ZERO); } @@ -189,7 +209,11 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.schedule(proposalId, Durations.ZERO); _proposals.execute(proposalId, Durations.ZERO); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Executed + ) + ); _proposals.execute(proposalId, Durations.ZERO); } @@ -199,7 +223,11 @@ contract ExecutableProposalsUnitTests is UnitTest { _proposals.schedule(proposalId, Durations.ZERO); _proposals.cancelAll(); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotScheduled.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.Cancelled + ) + ); _proposals.execute(proposalId, Durations.ZERO); } @@ -336,10 +364,18 @@ contract ExecutableProposalsUnitTests is UnitTest { } function testFuzz_get_not_existing_proposal(uint256 proposalId) external { - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotFound.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.NotExist + ) + ); _proposals.getProposalDetails(proposalId); - vm.expectRevert(abi.encodeWithSelector(ExecutableProposals.ProposalNotFound.selector, proposalId)); + vm.expectRevert( + abi.encodeWithSelector( + ExecutableProposals.UnexpectedProposalStatus.selector, proposalId, ProposalStatus.NotExist + ) + ); _proposals.getProposalCalls(proposalId); } diff --git a/test/unit/libraries/WithdrawalBatchesQueue.t.sol b/test/unit/libraries/WithdrawalBatchesQueue.t.sol index 4ea20861..b524a80c 100644 --- a/test/unit/libraries/WithdrawalBatchesQueue.t.sol +++ b/test/unit/libraries/WithdrawalBatchesQueue.t.sol @@ -16,7 +16,7 @@ contract WithdrawalsBatchesQueueTest is UnitTest { // --- function test_open_HappyPath() external { - assertEq(_batchesQueue.info.state, State.Absent); + assertEq(_batchesQueue.info.state, State.NotInitialized); assertEq(_batchesQueue.batches.length, 0); _batchesQueue.open(_DEFAULT_BOUNDARY_UNST_ETH_ID); @@ -31,12 +31,16 @@ contract WithdrawalsBatchesQueueTest is UnitTest { _batchesQueue.info.state = State.Opened; assertEq(_batchesQueue.info.state, State.Opened); - vm.expectRevert(WithdrawalsBatchesQueue.WithdrawalsBatchesQueueIsNotInAbsentState.selector); + vm.expectRevert( + abi.encodeWithSelector( + WithdrawalsBatchesQueue.UnexpectedWithdrawalsBatchesQueueState.selector, State.Opened + ) + ); _batchesQueue.open(_DEFAULT_BOUNDARY_UNST_ETH_ID); } function test_open_Emit_WithdrawalsBatchesQueueOpened() external { - assertEq(_batchesQueue.info.state, State.Absent); + assertEq(_batchesQueue.info.state, State.NotInitialized); vm.expectEmit(true, false, false, false); emit WithdrawalsBatchesQueue.WithdrawalsBatchesQueueOpened(_DEFAULT_BOUNDARY_UNST_ETH_ID); @@ -126,8 +130,22 @@ contract WithdrawalsBatchesQueueTest is UnitTest { ); } - function test_addUnstETHIds_RevertOn_QueueNotInOpenedState() external { - vm.expectRevert(WithdrawalsBatchesQueue.WithdrawalsBatchesQueueIsNotInOpenedState.selector); + function test_addUnstETHIds_RevertOn_QueueInNotInitializedState() external { + vm.expectRevert( + abi.encodeWithSelector( + WithdrawalsBatchesQueue.UnexpectedWithdrawalsBatchesQueueState.selector, State.NotInitialized + ) + ); + _batchesQueue.addUnstETHIds(new uint256[](0)); + } + + function test_addUnstETHIds_RevertOn_QueueInClosedState() external { + _batchesQueue.info.state = State.Closed; + vm.expectRevert( + abi.encodeWithSelector( + WithdrawalsBatchesQueue.UnexpectedWithdrawalsBatchesQueueState.selector, State.Closed + ) + ); _batchesQueue.addUnstETHIds(new uint256[](0)); } @@ -349,13 +367,21 @@ contract WithdrawalsBatchesQueueTest is UnitTest { } function test_close_RevertOn_QueueNotInOpenedState() external { - vm.expectRevert(WithdrawalsBatchesQueue.WithdrawalsBatchesQueueIsNotInOpenedState.selector); + vm.expectRevert( + abi.encodeWithSelector( + WithdrawalsBatchesQueue.UnexpectedWithdrawalsBatchesQueueState.selector, State.NotInitialized + ) + ); _batchesQueue.close(); _batchesQueue.open({boundaryUnstETHId: 1}); _batchesQueue.close(); - vm.expectRevert(WithdrawalsBatchesQueue.WithdrawalsBatchesQueueIsNotInOpenedState.selector); + vm.expectRevert( + abi.encodeWithSelector( + WithdrawalsBatchesQueue.UnexpectedWithdrawalsBatchesQueueState.selector, State.Closed + ) + ); _batchesQueue.close(); } @@ -551,43 +577,32 @@ contract WithdrawalsBatchesQueueTest is UnitTest { } // --- - // getLastClaimedOrBoundaryUnstETHId() + // getBoundaryUnstETHId() // --- - function test_getLastClaimedOrBoundaryUnstETHId_HappyPath_EmptyQueueReturnsBoundaryUnstETHId() external { + function test_getBoundaryUnstETHId_HappyPath_EmptyQueue() external { _openBatchesQueue(); - assertEq(_batchesQueue.getLastClaimedOrBoundaryUnstETHId(), _DEFAULT_BOUNDARY_UNST_ETH_ID); + _batchesQueue.close(); + assertEq(_batchesQueue.getBoundaryUnstETHId(), _DEFAULT_BOUNDARY_UNST_ETH_ID); } - function test_getLastClaimedOrBoundaryUnstETHId_HappyPath_NotEmptyQueueReturnsLastClaimedUnstETHId() external { + function test_getBoundaryUnstETHId_HappyPath_NotEmptyQueue() external { _openBatchesQueue(); + uint256 unstETHIdsCount = 5; uint256 firstUnstETHId = _DEFAULT_BOUNDARY_UNST_ETH_ID + 1; uint256[] memory unstETHIds = _generateFakeUnstETHIds({length: unstETHIdsCount, firstUnstETHId: firstUnstETHId}); _batchesQueue.addUnstETHIds(unstETHIds); assertEq(_batchesQueue.info.totalUnstETHIdsCount, 5); - assertEq(_batchesQueue.getLastClaimedOrBoundaryUnstETHId(), _DEFAULT_BOUNDARY_UNST_ETH_ID); - - uint256 maxUnstETHIdsCount = 3; - _batchesQueue.claimNextBatch(maxUnstETHIdsCount); - assertEq(_batchesQueue.getLastClaimedOrBoundaryUnstETHId(), unstETHIds[2]); - - _batchesQueue.claimNextBatch(maxUnstETHIdsCount); - assertEq(_batchesQueue.getLastClaimedOrBoundaryUnstETHId(), unstETHIds[unstETHIds.length - 1]); - } + _batchesQueue.close(); - function test_getLastClaimedOrBoundaryUnstETHId_RevertOn_AbsentQueueState() external { - vm.expectRevert(WithdrawalsBatchesQueue.WithdrawalsBatchesQueueIsInAbsentState.selector); - _batchesQueue.getLastClaimedOrBoundaryUnstETHId(); + assertEq(_batchesQueue.getBoundaryUnstETHId(), _DEFAULT_BOUNDARY_UNST_ETH_ID); } - function test_getLastClaimedOrBoundaryUnstETHId_RevertOn_LastClaimedBatchIndexOutOfArrayBounds() external { - _openBatchesQueue(); - _batchesQueue.info.lastClaimedBatchIndex = 2; - + function test_getBoundaryUnstETHId_RevertOn_NotInitializedQueueState() external { vm.expectRevert(stdError.indexOOBError); - _batchesQueue.getLastClaimedOrBoundaryUnstETHId(); + _batchesQueue.getBoundaryUnstETHId(); } // ---