Skip to content

Commit

Permalink
getTiebreakerDetails correct isTie value in edge case
Browse files Browse the repository at this point in the history
  • Loading branch information
@Psirex
Psirex committed Sep 27, 2024
1 parent 3a481f9 commit a3e6a8e
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 11 deletions.
7 changes: 1 addition & 6 deletions contracts/DualGovernance.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,12 +466,7 @@ contract DualGovernance is IDualGovernance {
/// (not in `Normal` or `VetoCooldown` state) before the tiebreaker committee is permitted to take actions.
/// - `sealableWithdrawalBlockers`: An array of sealable contracts registered in the system as withdrawal blockers.
function getTiebreakerDetails() external view returns (ITiebreaker.TiebreakerDetails memory tiebreakerState) {
return _tiebreaker.getTiebreakerDetails(
/// @dev Calling getEffectiveState() doesn't update the normalOrVetoCooldownStateExitedAt value,
/// but this does not distort the result of getTiebreakerDetails()
_stateMachine.getEffectiveState(),
_stateMachine.normalOrVetoCooldownExitedAt
);
return _tiebreaker.getTiebreakerDetails(_stateMachine.getStateDetails());
}

// ---
Expand Down
21 changes: 18 additions & 3 deletions contracts/libraries/Tiebreaker.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Timestamp, Timestamps} from "../types/Duration.sol";

import {ISealable} from "../interfaces/ISealable.sol";
import {ITiebreaker} from "../interfaces/ITiebreaker.sol";
import {IDualGovernance} from "../interfaces/IDualGovernance.sol";

import {SealableCalls} from "./SealableCalls.sol";
import {State as DualGovernanceState} from "./DualGovernanceStateMachine.sol";
Expand Down Expand Up @@ -190,15 +191,29 @@ library Tiebreaker {

/// @dev Retrieves the tiebreaker context from the storage.
/// @param self The storage context.
/// @param stateDetails A struct containing detailed information about the current state of the Dual Governance system
/// @return context The tiebreaker context containing the tiebreaker committee, tiebreaker activation timeout, and sealable withdrawal blockers.
function getTiebreakerDetails(
Context storage self,
DualGovernanceState state,
Timestamp normalOrVetoCooldownExitedAt
IDualGovernance.StateDetails memory stateDetails
) internal view returns (ITiebreaker.TiebreakerDetails memory context) {
context.tiebreakerCommittee = self.tiebreakerCommittee;
context.tiebreakerActivationTimeout = self.tiebreakerActivationTimeout;
context.isTie = isTie(self, state, normalOrVetoCooldownExitedAt);

DualGovernanceState persistedState = stateDetails.persistedState;
DualGovernanceState effectiveState = stateDetails.effectiveState;
Timestamp normalOrVetoCooldownExitedAt = stateDetails.normalOrVetoCooldownExitedAt;

if (effectiveState != persistedState) {
if (persistedState == DualGovernanceState.Normal || persistedState == DualGovernanceState.VetoCooldown) {
/// @dev When a pending state change is expected from the `Normal` or `VetoCooldown` state,
/// the `normalOrVetoCooldownExitedAt` timestamp should be set to the current timestamp to reflect
/// the behavior of the `DualGovernanceStateMachine.activateNextState()` method.
normalOrVetoCooldownExitedAt = Timestamps.now();
}
}

context.isTie = isTie(self, effectiveState, normalOrVetoCooldownExitedAt);

uint256 sealableWithdrawalBlockersCount = self.sealableWithdrawalBlockers.length();
context.sealableWithdrawalBlockers = new address[](sealableWithdrawalBlockersCount);
Expand Down
92 changes: 92 additions & 0 deletions test/unit/DualGovernance.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1879,6 +1879,98 @@ contract DualGovernanceUnitTests is UnitTest {
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);
}

function test_getTiebreakerDetails_NormalOrVetoCooldownExitedAtValueShouldBeUpdatedToCorrectlyCalculateIsTieValue()
external
{
address tiebreakerCommittee = makeAddr("TIEBREAKER_COMMITTEE");
address sealable = address(new SealableMock());
Duration tiebreakerActivationTimeout = Durations.from(180 days);

// for the correctness of the test, the following assumption must be true
assertTrue(tiebreakerActivationTimeout >= _configProvider.VETO_SIGNALLING_MAX_DURATION());

// setup tiebreaker

_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setTiebreakerCommittee.selector, tiebreakerCommittee)
);
_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.setTiebreakerActivationTimeout.selector, tiebreakerActivationTimeout)
);
_executor.execute(
address(_dualGovernance),
0,
abi.encodeWithSelector(DualGovernance.addTiebreakerSealableWithdrawalBlocker.selector, sealable)
);

assertEq(_dualGovernance.getPersistedState(), State.Normal);
assertEq(_dualGovernance.getEffectiveState(), State.Normal);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

vm.prank(vetoer);
_escrow.lockStETH(5 ether);

assertEq(_dualGovernance.getPersistedState(), State.VetoSignalling);
assertEq(_dualGovernance.getEffectiveState(), State.VetoSignalling);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

_wait(_configProvider.VETO_SIGNALLING_MAX_DURATION().plusSeconds(1));

assertEq(_dualGovernance.getPersistedState(), State.VetoSignalling);
assertEq(_dualGovernance.getEffectiveState(), State.RageQuit);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

_dualGovernance.activateNextState();

assertEq(_dualGovernance.getPersistedState(), State.RageQuit);
assertEq(_dualGovernance.getEffectiveState(), State.RageQuit);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

_wait(tiebreakerActivationTimeout);

vm.mockCall(
_dualGovernance.getRageQuitEscrow(),
abi.encodeWithSelector(Escrow.isRageQuitFinalized.selector),
abi.encode(true)
);

assertEq(_dualGovernance.getPersistedState(), State.RageQuit);
assertEq(_dualGovernance.getEffectiveState(), State.VetoCooldown);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

_dualGovernance.activateNextState();

assertEq(_dualGovernance.getPersistedState(), State.VetoCooldown);
assertEq(_dualGovernance.getEffectiveState(), State.VetoCooldown);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

// signalling accumulated rage quit support
vm.mockCall(
_dualGovernance.getVetoSignallingEscrow(),
abi.encodeWithSelector(Escrow.getRageQuitSupport.selector),
abi.encode(PercentsD16.fromBasisPoints(5_00))
);

_wait(_configProvider.VETO_COOLDOWN_DURATION().plusSeconds(1));

assertEq(_dualGovernance.getPersistedState(), State.VetoCooldown);
assertEq(_dualGovernance.getEffectiveState(), State.VetoSignalling);

// The extra case, when the transition from the VetoCooldown should happened.
// In such case, `normalOrVetoCooldownExitedAt` will be updated and isTie value
// still will be equal to `false`
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);

_dualGovernance.activateNextState();
assertEq(_dualGovernance.getPersistedState(), State.VetoSignalling);
assertEq(_dualGovernance.getEffectiveState(), State.VetoSignalling);
assertFalse(_dualGovernance.getTiebreakerDetails().isTie);
}

// ---
// resealSealable()
// ---
Expand Down
108 changes: 106 additions & 2 deletions test/unit/libraries/Tiebreaker.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {Tiebreaker} from "contracts/libraries/Tiebreaker.sol";
import {Duration, Durations, Timestamp, Timestamps} from "contracts/types/Duration.sol";
import {ISealable} from "contracts/interfaces/ISealable.sol";
import {ITiebreaker} from "contracts/interfaces/ITiebreaker.sol";
import {IDualGovernance} from "contracts/interfaces/IDualGovernance.sol";

import {UnitTest} from "test/utils/unit-test.sol";
import {SealableMock} from "../../mocks/SealableMock.sol";
Expand Down Expand Up @@ -190,8 +191,45 @@ contract TiebreakerTest is UnitTest {
context.tiebreakerActivationTimeout = timeout;
context.tiebreakerCommittee = address(0x123);

ITiebreaker.TiebreakerDetails memory details =
Tiebreaker.getTiebreakerDetails(context, DualGovernanceState.Normal, Timestamps.from(block.timestamp));
IDualGovernance.StateDetails memory stateDetails;
stateDetails.persistedState = DualGovernanceState.Normal;
stateDetails.effectiveState = DualGovernanceState.VetoSignalling;
stateDetails.normalOrVetoCooldownExitedAt = Timestamps.now();

ITiebreaker.TiebreakerDetails memory details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, false);
}

function test_getTiebreakerDetails_HappyPath_PendingTransitionFromVetoCooldown_ExpectIsTieFalse() external {
Tiebreaker.addSealableWithdrawalBlocker(context, address(mockSealable1), 1);

Duration timeout = Duration.wrap(5 days);

context.tiebreakerActivationTimeout = timeout;
context.tiebreakerCommittee = address(0x123);

IDualGovernance.StateDetails memory stateDetails;

stateDetails.persistedState = DualGovernanceState.VetoCooldown;
stateDetails.effectiveState = DualGovernanceState.VetoSignalling;
stateDetails.normalOrVetoCooldownExitedAt = Timestamps.now();

ITiebreaker.TiebreakerDetails memory details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, false);

_wait(timeout);

details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
Expand All @@ -200,6 +238,72 @@ contract TiebreakerTest is UnitTest {
assertEq(details.isTie, false);
}

function test_getTiebreakerDetails_HappyPath_PendingTransitionFromNormal_ExpectIsTieFalse() external {
Tiebreaker.addSealableWithdrawalBlocker(context, address(mockSealable1), 1);

Duration timeout = Duration.wrap(5 days);

context.tiebreakerActivationTimeout = timeout;
context.tiebreakerCommittee = address(0x123);

IDualGovernance.StateDetails memory stateDetails;

stateDetails.persistedState = DualGovernanceState.Normal;
stateDetails.effectiveState = DualGovernanceState.VetoSignalling;
stateDetails.normalOrVetoCooldownExitedAt = Timestamps.now();

ITiebreaker.TiebreakerDetails memory details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, false);

_wait(timeout);

details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, false);
}

function test_getTiebreakerDetails_HappyPath_PendingTransitionFromVetoSignalling_ExpectIsTieTrue() external {
Tiebreaker.addSealableWithdrawalBlocker(context, address(mockSealable1), 1);

Duration timeout = Duration.wrap(5 days);

context.tiebreakerActivationTimeout = timeout;
context.tiebreakerCommittee = address(0x123);

IDualGovernance.StateDetails memory stateDetails;

stateDetails.persistedState = DualGovernanceState.VetoSignalling;
stateDetails.effectiveState = DualGovernanceState.RageQuit;
stateDetails.normalOrVetoCooldownExitedAt = Timestamps.now();

ITiebreaker.TiebreakerDetails memory details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, false);

_wait(timeout);

details = Tiebreaker.getTiebreakerDetails(context, stateDetails);

assertEq(details.tiebreakerCommittee, context.tiebreakerCommittee);
assertEq(details.tiebreakerActivationTimeout, context.tiebreakerActivationTimeout);
assertEq(details.sealableWithdrawalBlockers[0], address(mockSealable1));
assertEq(details.sealableWithdrawalBlockers.length, 1);
assertEq(details.isTie, true);
}

function external__checkCallerIsTiebreakerCommittee() external view {
Tiebreaker.checkCallerIsTiebreakerCommittee(context);
}
Expand Down

0 comments on commit a3e6a8e

Please sign in to comment.