From d97c42306e04cc1d4c96a65587498233c0270337 Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 29 Aug 2024 12:21:40 +0200 Subject: [PATCH 1/5] Add sendToCustodian function to VestingWalletPaused --- src/L1/paused/L1VestingWalletPaused.sol | 6 +++- src/L2/paused/L2VestingWalletPaused.sol | 13 ++++++++- test/L2/paused/L2VestingWalletPaused.t.sol | 34 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/L1/paused/L1VestingWalletPaused.sol b/src/L1/paused/L1VestingWalletPaused.sol index 4039b005..805b0525 100644 --- a/src/L1/paused/L1VestingWalletPaused.sol +++ b/src/L1/paused/L1VestingWalletPaused.sol @@ -8,4 +8,8 @@ import { L2VestingWalletPaused } from "src/L2/paused/L2VestingWalletPaused.sol"; /// and /// pause the contract to prevent any further vesting operations. /// L1VestingWalletPaused shares the same functionality of L1VestingWalletPaused. -contract L1VestingWalletPaused is L2VestingWalletPaused { } +contract L1VestingWalletPaused is L2VestingWalletPaused { + function custodianAddress() public pure virtual override returns (address) { + return 0xD2D7535e099F26EbfbA26d96bD1a661d3531d0e9; + } +} diff --git a/src/L2/paused/L2VestingWalletPaused.sol b/src/L2/paused/L2VestingWalletPaused.sol index e2ed47ea..5bc6a73b 100644 --- a/src/L2/paused/L2VestingWalletPaused.sol +++ b/src/L2/paused/L2VestingWalletPaused.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity 0.8.23; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { L2VestingWallet } from "src/L2/L2VestingWallet.sol"; /// @title L2VestingWalletPaused - Paused version of L2VestingWallet contract @@ -12,7 +13,17 @@ contract L2VestingWalletPaused is L2VestingWallet { /// @notice Setting global params. function initializePaused() public reinitializer(2) { - version = "1.0.0-paused"; + version = "1.0.1-paused"; + } + + /// @notice Hard-coded address to recover tokens. + function custodianAddress() public pure virtual returns (address) { + return 0x394Ae9d48eeca1C69a989B5A8C787081595c55A7; + } + + /// @notice Withdraw all balances of token to recoveryAddress. + function sendToCustodian(IERC20 _token) public virtual onlyRole(CONTRACT_ADMIN_ROLE) { + _token.transfer(custodianAddress(), _token.balanceOf(address(this))); } /// @notice Override the release function to prevent release of token from being processed. diff --git a/test/L2/paused/L2VestingWalletPaused.t.sol b/test/L2/paused/L2VestingWalletPaused.t.sol index f2bab1fb..39c9f7d1 100644 --- a/test/L2/paused/L2VestingWalletPaused.t.sol +++ b/test/L2/paused/L2VestingWalletPaused.t.sol @@ -2,6 +2,8 @@ pragma solidity 0.8.23; import { Test, console, stdJson } from "forge-std/Test.sol"; +import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { IAccessControl } from "@openzeppelin/contracts/access/IAccessControl.sol"; import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; import { L2VestingWallet } from "src/L2/L2VestingWallet.sol"; import { L2VestingWalletPaused } from "src/L2/paused/L2VestingWalletPaused.sol"; @@ -22,6 +24,8 @@ contract L2VestingWalletPausedTest is Test { L2VestingWallet public l2VestingWallet; L2VestingWallet public l2VestingWalletImplementation; + + L2VestingWalletPaused public l2VestingWalletPaused; L2VestingWalletPaused public l2VestingWalletPausedImplementation; MockERC20 public mockToken; @@ -72,6 +76,36 @@ contract L2VestingWalletPausedTest is Test { abi.encodeWithSelector(l2VestingWalletPausedImplementation.initializePaused.selector) ); vm.stopPrank(); + + // Wrapping with L2VestingWalletPaused + l2VestingWalletPaused = L2VestingWalletPaused(payable(address(l2VestingWallet))); + } + + function test_SendToCustodian_RevertWhenNotContractAdmin() public { + address nobody = vm.addr(1); + + vm.startPrank(nobody); + vm.expectRevert( + abi.encodeWithSelector( + IAccessControl.AccessControlUnauthorizedAccount.selector, nobody, l2VestingWallet.CONTRACT_ADMIN_ROLE() + ) + ); + l2VestingWalletPaused.sendToCustodian(IERC20(address(mockToken))); + vm.stopPrank(); + } + + function test_SendToCustodian() public { + uint256 contractBalanceBefore = mockToken.balanceOf(address(l2VestingWalletPaused)); + uint256 recoveryAddressBalanceBefore = mockToken.balanceOf(l2VestingWalletPaused.custodianAddress()); + + vm.startPrank(contractAdmin); + l2VestingWalletPaused.sendToCustodian(IERC20(address(mockToken))); + vm.stopPrank(); + + assertEq( + mockToken.balanceOf(l2VestingWalletPaused.custodianAddress()), + contractBalanceBefore + recoveryAddressBalanceBefore + ); } function test_ReleaseWithAddress_Paused() public { From 98aa8a1ea53598d30816a0992ba9fc098abec795 Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 29 Aug 2024 14:30:35 +0200 Subject: [PATCH 2/5] Updated fmt --- src/L2/L2Governor.sol | 8 ++------ src/L2/L2LockingPosition.sol | 12 +++--------- src/L2/L2Staking.sol | 8 ++------ src/L2/L2VestingWallet.sol | 8 ++------ src/interfaces/L2/IL2Governor.sol | 4 +--- src/interfaces/L2/IL2LockingPosition.sol | 4 +--- test/L2/L2LockingPosition.t.sol | 4 +--- test/L2/L2Staking.t.sol | 4 +--- 8 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/L2/L2Governor.sol b/src/L2/L2Governor.sol index 03f7ff2d..56a38752 100644 --- a/src/L2/L2Governor.sol +++ b/src/L2/L2Governor.sol @@ -89,9 +89,7 @@ contract L2Governor is // The below functions are overrides required by Solidity. - function state( - uint256 proposalId - ) + function state(uint256 proposalId) public view virtual @@ -101,9 +99,7 @@ contract L2Governor is return super.state(proposalId); } - function proposalNeedsQueuing( - uint256 proposalId - ) + function proposalNeedsQueuing(uint256 proposalId) public view virtual diff --git a/src/L2/L2LockingPosition.sol b/src/L2/L2LockingPosition.sol index 3288ffcd..24780ba8 100644 --- a/src/L2/L2LockingPosition.sol +++ b/src/L2/L2LockingPosition.sol @@ -93,9 +93,7 @@ contract L2LockingPosition is Initializable, Ownable2StepUpgradeable, UUPSUpgrad /// initialized to 0 or address(0). /// @param position Locking position to be checked. /// @return Whether the given locking position is null. - function isLockingPositionNull( - IL2LockingPosition.LockingPosition memory position - ) + function isLockingPositionNull(IL2LockingPosition.LockingPosition memory position) internal view virtual @@ -266,9 +264,7 @@ contract L2LockingPosition is Initializable, Ownable2StepUpgradeable, UUPSUpgrad /// @notice Returns the locking position for the given position ID. /// @param positionId ID of the locking position. /// @return Locking position for the given position ID. - function getLockingPosition( - uint256 positionId - ) + function getLockingPosition(uint256 positionId) public view virtual @@ -280,9 +276,7 @@ contract L2LockingPosition is Initializable, Ownable2StepUpgradeable, UUPSUpgrad /// @notice Returns all locking positions for the given owner. /// @param lockOwner Owner address. /// @return All locking positions for the given owner. - function getAllLockingPositionsByOwner( - address lockOwner - ) + function getAllLockingPositionsByOwner(address lockOwner) public view virtual diff --git a/src/L2/L2Staking.sol b/src/L2/L2Staking.sol index f413da8a..45d534d2 100644 --- a/src/L2/L2Staking.sol +++ b/src/L2/L2Staking.sol @@ -119,9 +119,7 @@ contract L2Staking is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, I /// initialized to 0 or address(0). /// @param position Locking position to be checked. /// @return Whether the given locking position is null. - function isLockingPositionNull( - IL2LockingPosition.LockingPosition memory position - ) + function isLockingPositionNull(IL2LockingPosition.LockingPosition memory position) internal view virtual @@ -187,9 +185,7 @@ contract L2Staking is Initializable, Ownable2StepUpgradeable, UUPSUpgradeable, I /// @notice Returns the remaining locking duration for the given locking position. /// @param lock The locking position for which the remaining locking duration is returned. /// @return The remaining locking duration for the given locking position. - function remainingLockingDuration( - IL2LockingPosition.LockingPosition memory lock - ) + function remainingLockingDuration(IL2LockingPosition.LockingPosition memory lock) internal view virtual diff --git a/src/L2/L2VestingWallet.sol b/src/L2/L2VestingWallet.sol index ac429faf..dbf8a7be 100644 --- a/src/L2/L2VestingWallet.sol +++ b/src/L2/L2VestingWallet.sol @@ -65,9 +65,7 @@ contract L2VestingWallet is /// @notice Since `Ownable2StepUpgradeable` is enforced on top of `OwnableUpgradeable`. Overriding is required. /// @param _newOwner New proposed owner. - function transferOwnership( - address _newOwner - ) + function transferOwnership(address _newOwner) public virtual override(Ownable2StepUpgradeable, OwnableUpgradeable) @@ -78,9 +76,7 @@ contract L2VestingWallet is // Overriding _transferOwnership and solely uses `Ownable2StepUpgradeable`. /// @param _newOwner New proposed owner. - function _transferOwnership( - address _newOwner - ) + function _transferOwnership(address _newOwner) internal virtual override(Ownable2StepUpgradeable, OwnableUpgradeable) diff --git a/src/interfaces/L2/IL2Governor.sol b/src/interfaces/L2/IL2Governor.sol index 9d14f2d2..b122a1a7 100644 --- a/src/interfaces/L2/IL2Governor.sol +++ b/src/interfaces/L2/IL2Governor.sol @@ -174,9 +174,7 @@ interface IL2Governor { function proposalProposer(uint256 proposalId) external view returns (address); function proposalSnapshot(uint256 proposalId) external view returns (uint256); function proposalThreshold() external view returns (uint256); - function proposalVotes( - uint256 proposalId - ) + function proposalVotes(uint256 proposalId) external view returns (uint256 againstVotes, uint256 forVotes, uint256 abstainVotes); diff --git a/src/interfaces/L2/IL2LockingPosition.sol b/src/interfaces/L2/IL2LockingPosition.sol index 00ba5ce7..be51ff9b 100644 --- a/src/interfaces/L2/IL2LockingPosition.sol +++ b/src/interfaces/L2/IL2LockingPosition.sol @@ -67,9 +67,7 @@ interface IL2LockingPosition { function initialize(address _stakingContract) external; function initializeVotingPower(address _votingPowerContract) external; function isApprovedForAll(address owner, address operator) external view returns (bool); - function lockingPositions( - uint256 - ) + function lockingPositions(uint256) external view returns (address creator, uint256 amount, uint256 expDate, uint256 pausedLockingDuration); diff --git a/test/L2/L2LockingPosition.t.sol b/test/L2/L2LockingPosition.t.sol index 120ce720..aa7168f4 100644 --- a/test/L2/L2LockingPosition.t.sol +++ b/test/L2/L2LockingPosition.t.sol @@ -27,9 +27,7 @@ contract L2LockingPositionV2 is L2LockingPosition { } contract L2LockingPositionHarness is L2LockingPosition { - function exposedIsLockingPositionNull( - IL2LockingPosition.LockingPosition memory position - ) + function exposedIsLockingPositionNull(IL2LockingPosition.LockingPosition memory position) public view returns (bool) diff --git a/test/L2/L2Staking.t.sol b/test/L2/L2Staking.t.sol index fa99e381..63bb3462 100644 --- a/test/L2/L2Staking.t.sol +++ b/test/L2/L2Staking.t.sol @@ -41,9 +41,7 @@ contract L2StakingHarness is L2Staking { return canLockingPositionBeModified(lockId, lock); } - function exposedRemainingLockingDuration( - IL2LockingPosition.LockingPosition memory lock - ) + function exposedRemainingLockingDuration(IL2LockingPosition.LockingPosition memory lock) public view returns (uint256) From 34508ff34b165d1e6042680e848d531650f2db0a Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 29 Aug 2024 14:41:28 +0200 Subject: [PATCH 3/5] Use SafeERC20 --- src/L2/paused/L2VestingWalletPaused.sol | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/L2/paused/L2VestingWalletPaused.sol b/src/L2/paused/L2VestingWalletPaused.sol index 5bc6a73b..550932ad 100644 --- a/src/L2/paused/L2VestingWalletPaused.sol +++ b/src/L2/paused/L2VestingWalletPaused.sol @@ -2,13 +2,15 @@ pragma solidity 0.8.23; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import { L2VestingWallet } from "src/L2/L2VestingWallet.sol"; /// @title L2VestingWalletPaused - Paused version of L2VestingWallet contract /// @notice This contract is used to pause the L2VestingWallet contract. In case of any emergency, the owner can upgrade -/// and -/// pause the contract to prevent any further vesting operations. +/// and pause the contract to prevent any further vesting operations. contract L2VestingWalletPaused is L2VestingWallet { + using SafeERC20 for IERC20; + error VestingWalletIsPaused(); /// @notice Setting global params. @@ -23,7 +25,7 @@ contract L2VestingWalletPaused is L2VestingWallet { /// @notice Withdraw all balances of token to recoveryAddress. function sendToCustodian(IERC20 _token) public virtual onlyRole(CONTRACT_ADMIN_ROLE) { - _token.transfer(custodianAddress(), _token.balanceOf(address(this))); + _token.safeTransfer(custodianAddress(), _token.balanceOf(address(this))); } /// @notice Override the release function to prevent release of token from being processed. From ed4f2caaf50ea7beb957eab1e80b70f9793f8209 Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 29 Aug 2024 15:12:03 +0200 Subject: [PATCH 4/5] Added comments to VestingWallets --- src/L1/paused/L1VestingWalletPaused.sol | 4 ++-- src/L2/paused/L2VestingWalletPaused.sol | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/L1/paused/L1VestingWalletPaused.sol b/src/L1/paused/L1VestingWalletPaused.sol index 805b0525..10c1975b 100644 --- a/src/L1/paused/L1VestingWalletPaused.sol +++ b/src/L1/paused/L1VestingWalletPaused.sol @@ -5,11 +5,11 @@ import { L2VestingWalletPaused } from "src/L2/paused/L2VestingWalletPaused.sol"; /// @title L1VestingWalletPaused - Paused version of L1VestingWallet contract /// @notice This contract is used to pause the L1VestingWallet contract. In case of any emergency, the owner can upgrade -/// and -/// pause the contract to prevent any further vesting operations. +/// and pause the contract to prevent any further vesting operations. /// L1VestingWalletPaused shares the same functionality of L1VestingWalletPaused. contract L1VestingWalletPaused is L2VestingWalletPaused { function custodianAddress() public pure virtual override returns (address) { + // Address of Security Council on L1 return 0xD2D7535e099F26EbfbA26d96bD1a661d3531d0e9; } } diff --git a/src/L2/paused/L2VestingWalletPaused.sol b/src/L2/paused/L2VestingWalletPaused.sol index 550932ad..6cd11560 100644 --- a/src/L2/paused/L2VestingWalletPaused.sol +++ b/src/L2/paused/L2VestingWalletPaused.sol @@ -6,8 +6,10 @@ import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.s import { L2VestingWallet } from "src/L2/L2VestingWallet.sol"; /// @title L2VestingWalletPaused - Paused version of L2VestingWallet contract -/// @notice This contract is used to pause the L2VestingWallet contract. In case of any emergency, the owner can upgrade +/// @notice This contract is used to pause the L2VestingWallet contract. In case of any emergency, the contract admin +/// can upgrade /// and pause the contract to prevent any further vesting operations. +/// Contract admin can also withdraw all tokens to the Security Council wallet. contract L2VestingWalletPaused is L2VestingWallet { using SafeERC20 for IERC20; @@ -20,6 +22,7 @@ contract L2VestingWalletPaused is L2VestingWallet { /// @notice Hard-coded address to recover tokens. function custodianAddress() public pure virtual returns (address) { + // Address of Security Council on L2 return 0x394Ae9d48eeca1C69a989B5A8C787081595c55A7; } From c6bfae4b27c023b1f6a3385a56aec23f8586aced Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 29 Aug 2024 15:21:59 +0200 Subject: [PATCH 5/5] Add custodianAddress check --- test/L1/paused/L1VestingWalletPaused.t.sol | 76 ++++++++++++++++++++++ test/L2/paused/L2VestingWalletPaused.t.sol | 5 ++ 2 files changed, 81 insertions(+) create mode 100644 test/L1/paused/L1VestingWalletPaused.t.sol diff --git a/test/L1/paused/L1VestingWalletPaused.t.sol b/test/L1/paused/L1VestingWalletPaused.t.sol new file mode 100644 index 00000000..475880fb --- /dev/null +++ b/test/L1/paused/L1VestingWalletPaused.t.sol @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 +pragma solidity 0.8.23; + +import { Test, console, stdJson } from "forge-std/Test.sol"; +import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; +import { L1VestingWallet } from "src/L1/L1VestingWallet.sol"; +import { L1VestingWalletPaused } from "src/L1/paused/L1VestingWalletPaused.sol"; +import { MockERC20 } from "test/mock/MockERC20.sol"; + +contract L1VestingWalletPausedTest is Test { + using stdJson for string; + + L1VestingWallet public l1VestingWallet; + L1VestingWallet public l1VestingWalletImplementation; + + L1VestingWalletPaused public l1VestingWalletPaused; + L1VestingWalletPaused public l1VestingWalletPausedImplementation; + + MockERC20 public mockToken; + + address public beneficiary = vm.addr(uint256(bytes32("beneficiary"))); + address public contractAdmin = vm.addr(uint256(bytes32("contractAdmin"))); + uint64 public startTimestamp = uint64(vm.getBlockTimestamp()); + uint64 public durationSeconds = 1000; + string public name = "Vesting Wallet"; + + uint256 public vestAmount = 1_000_000; + + function setUp() public { + // deploy L1VestingWallet implementation contract + l1VestingWalletImplementation = new L1VestingWallet(); + + // deploy L1VestingWallet contract via proxy and initialize it at the same time + l1VestingWallet = L1VestingWallet( + payable( + address( + new ERC1967Proxy( + address(l1VestingWalletImplementation), + abi.encodeWithSelector( + l1VestingWalletImplementation.initialize.selector, + beneficiary, + startTimestamp, + durationSeconds, + name, + contractAdmin + ) + ) + ) + ) + ); + assert(address(l1VestingWallet) != address(0x0)); + + // transfer token to vesting contract + mockToken = new MockERC20(vestAmount); + mockToken.transfer(address(l1VestingWallet), vestAmount); + + // deploy L1VestingWalletPaused implementation contract + l1VestingWalletPausedImplementation = new L1VestingWalletPaused(); + + // upgrade L1VestingWallet contract to L1VestingWalletPaused contract + vm.startPrank(contractAdmin); + l1VestingWallet.upgradeToAndCall( + address(l1VestingWalletPausedImplementation), + abi.encodeWithSelector(l1VestingWalletPausedImplementation.initializePaused.selector) + ); + vm.stopPrank(); + + // Wrapping with L1VestingWalletPaused + l1VestingWalletPaused = L1VestingWalletPaused(payable(address(l1VestingWallet))); + } + + function test_CustodianAddress() public view { + // Address of Security Council on L1 + assertEq(l1VestingWalletPaused.custodianAddress(), 0xD2D7535e099F26EbfbA26d96bD1a661d3531d0e9); + } +} diff --git a/test/L2/paused/L2VestingWalletPaused.t.sol b/test/L2/paused/L2VestingWalletPaused.t.sol index 39c9f7d1..135c08cf 100644 --- a/test/L2/paused/L2VestingWalletPaused.t.sol +++ b/test/L2/paused/L2VestingWalletPaused.t.sol @@ -81,6 +81,11 @@ contract L2VestingWalletPausedTest is Test { l2VestingWalletPaused = L2VestingWalletPaused(payable(address(l2VestingWallet))); } + function test_CustodianAddress() public view { + // Address of Security Council on L2 + assertEq(l2VestingWalletPaused.custodianAddress(), 0x394Ae9d48eeca1C69a989B5A8C787081595c55A7); + } + function test_SendToCustodian_RevertWhenNotContractAdmin() public { address nobody = vm.addr(1);