Skip to content

Commit

Permalink
feat: timelock test
Browse files Browse the repository at this point in the history
  • Loading branch information
johnnyonline committed Jun 11, 2024
1 parent ba31476 commit 6e0331b
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 1 deletion.
1 change: 1 addition & 0 deletions .env.sample
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
ETHEREUM_RPC_URL=YOUR_ETH_RPC_URL
PRIVATE_KEY=YOUR_PRIVATE_KEY
INFURA_PROJECT_ID=YOUR_INFURA_PROJECT_ID
ETHERSCAN_API_KEY=YOUR_ETHERSCAN_API_KEY
Expand Down
3 changes: 2 additions & 1 deletion test/scenarios/ScenarioBaseTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {IDelegationManager} from "lib/eigenlayer-contracts/src/contracts/interfa
import {IStakingNodesManager} from "src/interfaces/IStakingNodesManager.sol";
import {IRewardsDistributor} from "src/interfaces/IRewardsDistributor.sol";
import {IynETH} from "src/interfaces/IynETH.sol";
import {Test} from "forge-std/Test.sol";
import {ynETH} from "src/ynETH.sol";
import {ynLSD} from "src/ynLSD.sol";
import {YieldNestOracle} from "src/YieldNestOracle.sol";
Expand All @@ -29,6 +28,8 @@ import {Utils} from "script/Utils.sol";
import {ActorAddresses} from "script/Actors.sol";
import {TestAssetUtils} from "test/utils/TestAssetUtils.sol";

import "forge-std/Test.sol";

contract ScenarioBaseTest is Test, Utils {

// Utils
Expand Down
114 changes: 114 additions & 0 deletions test/scenarios/Timelock.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
// SPDX-License-Identifier: BSD-3-Clause
pragma solidity ^0.8.24;

import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {TimelockController} from "@openzeppelin/contracts/governance/TimelockController.sol";

import "./ScenarioBaseTest.sol";

contract TimelockTest is ScenarioBaseTest {

event Upgraded(address indexed implementation);

uint256 public constant DELAY = 3 days;

address[] public proxyContracts;

TimelockController public timelock;

// ============================================================================================
// Setup
// ============================================================================================

function setUp() public override {
vm.selectFork(vm.createFork(vm.envString("ETHEREUM_RPC_URL")));

ScenarioBaseTest.setUp();

proxyContracts = [
address(yneth),
address(stakingNodesManager),
address(rewardsDistributor),
address(executionLayerReceiver),
address(consensusLayerReceiver)
];

address[] memory _proposers = new address[](1);

This comment has been minimized.

Copy link
@danoctavian

danoctavian Jun 17, 2024

Contributor

given the 5 signers of the Security Council multisig now

Who do we set for proposers, and who do we set as executors?

This comment has been minimized.

Copy link
@johnnyonline

johnnyonline Jun 17, 2024

Author Member

proposers can be EOAs.

for executors we'll probably want the most robust msigs.

_proposers[0] = actors.eoa.DEFAULT_SIGNER;
address[] memory _executors = new address[](1);
_executors[0] = actors.eoa.DEFAULT_SIGNER;
timelock = new TimelockController(
DELAY,
_proposers,
_executors,
address(0) // no admin

This comment has been minimized.

Copy link
@danoctavian

danoctavian Jun 17, 2024

Contributor

Why no admin?

Suppose that we want to change the PROPOSERS or the EXECUTORS how is that achieved?

This comment has been minimized.

Copy link
@johnnyonline

johnnyonline Jun 17, 2024

Author Member

yea definitely can populate this role. probably with the most robust msig.

the default admin will be able to change the PROPOSERS/EXECUTORS.

);
}

// ============================================================================================
// Tests
// ============================================================================================

function testScheduleAndExecuteUpgrade() public {
_updateProxyAdminOwnersToTimelock();

// operation data
address _target = getTransparentUpgradeableProxyAdminAddress(address(yneth)); // proxy admin
address _implementation = getTransparentUpgradeableProxyImplementationAddress(address(yneth)); // implementation (not changed)
uint256 _value = 0;
bytes memory _data = abi.encodeWithSignature(
"upgradeAndCall(address,address,bytes)",
address(yneth), // proxy
_implementation, // implementation
"" // no data
);
bytes32 _predecessor = bytes32(0);
bytes32 _salt = bytes32(0);
uint256 _delay = 3 days;

vm.startPrank(actors.eoa.DEFAULT_SIGNER);

// schedule
timelock.schedule(
_target,
_value,
_data,
_predecessor,
_salt,
_delay
);

// skip delay duration
skip(DELAY);

vm.expectEmit(address(yneth));
emit Upgraded(_implementation);

// execute
timelock.execute(
_target,
_value,
_data,
_predecessor,
_salt
);

vm.stopPrank();

This comment has been minimized.

Copy link
@danoctavian

danoctavian Jun 17, 2024

Contributor

Demonstrate how you replace the timelock with another arbitrary admin again.

(Can still swap ownership after this operation)

This comment has been minimized.

Copy link
@johnnyonline

johnnyonline Jun 18, 2024

Author Member
}

// ============================================================================================
// Internal helpers
// ============================================================================================

function _updateProxyAdminOwnersToTimelock() internal {

This comment has been minimized.

Copy link
@danoctavian

danoctavian Jun 17, 2024

Contributor

Are timelocks necessary only for upgrade control?

Can you think of any other use cases in general and for our roles?

This comment has been minimized.

Copy link
@johnnyonline

johnnyonline Jun 17, 2024

Author Member

there are some privileged roles that maybe could be nice to put behind a timelock, or make deterministic, like the WITHDRAWER_ROLE in the RewardsReceiver contracts, as they can specify a receiver when withdrawing assets.

This comment has been minimized.

Copy link
@danoctavian

danoctavian Jun 17, 2024

Contributor

@johnnyonline in prod those roles are configured to be RewardsDistributor so in that case it's always that specific contract.

Therefore in that case it's not needed!

for (uint256 i = 0; i < proxyContracts.length; i++) {

// get proxy admin
Ownable _proxyAdmin = Ownable(getTransparentUpgradeableProxyAdminAddress(address(proxyContracts[i])));

// transfer ownership to timelock
vm.prank(_proxyAdmin.owner());
_proxyAdmin.transferOwnership(address(timelock));
}
}
}

0 comments on commit 6e0331b

Please sign in to comment.