diff --git a/.yarn/install-state.gz b/.yarn/install-state.gz new file mode 100644 index 0000000..211623a Binary files /dev/null and b/.yarn/install-state.gz differ diff --git a/src/contracts/StakingContract.sol b/src/contracts/StakingContract.sol index 7f1d45d..47c1f9f 100644 --- a/src/contracts/StakingContract.sol +++ b/src/contracts/StakingContract.sol @@ -53,14 +53,6 @@ contract StakingContract { error AddressSanctioned(address sanctionedAccount); error AddressBlocked(address blockedAccount); - struct ValidatorAllocationCache { - bool used; - uint8 operatorIndex; - uint32 funded; - uint32 toDeposit; - uint32 available; - } - event Deposit(address indexed caller, address indexed withdrawer, bytes publicKey, bytes signature); event ValidatorKeysAdded(uint256 indexed operatorIndex, bytes publicKeys, bytes signatures); event ValidatorKeyRemoved(uint256 indexed operatorIndex, bytes publicKey); @@ -75,9 +67,10 @@ contract StakingContract { event ChangedOperatorAddresses(uint256 operatorIndex, address operatorAddress, address feeRecipientAddress); event DeactivatedOperator(uint256 _operatorIndex); event ActivatedOperator(uint256 _operatorIndex); - event SetWithdrawerCustomizationStatus(bool _status); event ExitRequest(address caller, bytes pubkey); event ValidatorsEdited(uint256 blockNumber); + event NewSanctionsOracle(address sanctionsOracle); + event BeginOwnershipTransfer(address indexed previousAdmin, address indexed newAdmin); /// @notice Ensures an initialisation call has been called only once per _version value /// @param _version The current initialisation value @@ -199,18 +192,19 @@ contract StakingContract { StakingContractStorageLib.setOperatorCommissionLimit(operatorCommissionLimitBPS); } - /// @notice Changes the behavior of the withdrawer customization logic - /// @param _enabled True to allow users to customize the withdrawer - function setWithdrawerCustomizationEnabled(bool _enabled) external onlyAdmin { - StakingContractStorageLib.setWithdrawerCustomizationEnabled(_enabled); - emit SetWithdrawerCustomizationStatus(_enabled); - } - /// @notice Changes the sanctions oracle address /// @param _sanctionsOracle New sanctions oracle address /// @dev If the address is address(0), the sanctions oracle checks are skipped function setSanctionsOracle(address _sanctionsOracle) external onlyAdmin { StakingContractStorageLib.setSanctionsOracle(_sanctionsOracle); + emit NewSanctionsOracle(_sanctionsOracle); + } + + /// @notice Get the sanctions oracle address + /// @notice If the address is address(0), the sanctions oracle checks are skipped + /// @return sanctionsOracle The sanctions oracle address + function getSanctionsOracle() external view returns (address) { + return StakingContractStorageLib.getSanctionsOracle(); } /// @notice Retrieve system admin @@ -274,9 +268,20 @@ contract StakingContract { /// @notice Retrieve withdrawer of public key root /// @notice In case the validator is not enabled, it will return address(0) + /// @notice In case the owner of the validator is sanctioned, it will revert /// @param _publicKeyRoot Hash of the public key function getWithdrawerFromPublicKeyRoot(bytes32 _publicKeyRoot) external view returns (address) { - return _getWithdrawer(_publicKeyRoot); + address withdrawer = _getWithdrawer(_publicKeyRoot); + if (withdrawer == address(0)) { + return address(0); + } + address sanctionsOracle = StakingContractStorageLib.getSanctionsOracle(); + if (sanctionsOracle != address(0)) { + if (ISanctionsOracle(sanctionsOracle).isSanctioned(withdrawer)) { + revert AddressSanctioned(withdrawer); + } + } + return withdrawer; } /// @notice Retrieve whether the validator exit has been requested @@ -375,6 +380,7 @@ contract StakingContract { /// @param _newAdmin New Administrator address function transferOwnership(address _newAdmin) external onlyAdmin { StakingContractStorageLib.setPendingAdmin(_newAdmin); + emit BeginOwnershipTransfer(msg.sender, _newAdmin); } /// @notice New admin must accept its role by calling this method @@ -433,27 +439,6 @@ contract StakingContract { emit ChangedOperatorAddresses(_operatorIndex, _operatorAddress, _feeRecipientAddress); } - /// @notice Set withdrawer for public key - /// @dev Only callable by current public key withdrawer - /// @param _publicKey Public key to change withdrawer - /// @param _newWithdrawer New withdrawer address - function setWithdrawer(bytes calldata _publicKey, address _newWithdrawer) external { - if (!StakingContractStorageLib.getWithdrawerCustomizationEnabled()) { - revert Forbidden(); - } - _checkAddress(_newWithdrawer); - bytes32 pubkeyRoot = _getPubKeyRoot(_publicKey); - StakingContractStorageLib.WithdrawersSlot storage withdrawers = StakingContractStorageLib.getWithdrawers(); - - if (withdrawers.value[pubkeyRoot] != msg.sender) { - revert Unauthorized(); - } - - emit ChangedWithdrawer(_publicKey, _newWithdrawer); - - withdrawers.value[pubkeyRoot] = _newWithdrawer; - } - /// @notice Set operator staking limits /// @dev Only callable by admin /// @dev Limit should not exceed the validator key count of the operator @@ -547,11 +532,11 @@ contract StakingContract { revert InvalidArgument(); } - if (_publicKeys.length % PUBLIC_KEY_LENGTH != 0 || _publicKeys.length / PUBLIC_KEY_LENGTH != _keyCount) { + if (_publicKeys.length != PUBLIC_KEY_LENGTH * _keyCount) { revert InvalidPublicKeys(); } - if (_signatures.length % SIGNATURE_LENGTH != 0 || _signatures.length / SIGNATURE_LENGTH != _keyCount) { + if (_signatures.length != SIGNATURE_LENGTH * _keyCount) { revert InvalidSignatures(); } @@ -967,13 +952,6 @@ contract StakingContract { _depositOnOneOperator(depositCount, totalAvailableValidators); } - function _min(uint256 _a, uint256 _b) internal pure returns (uint256) { - if (_a < _b) { - return _a; - } - return _b; - } - /// @notice Internal utility to compute the receiver deterministic address /// @param _publicKey Public Key assigned to the receiver /// @param _prefix Prefix used to generate multiple receivers per public key diff --git a/src/contracts/libs/StakingContractStorageLib.sol b/src/contracts/libs/StakingContractStorageLib.sol index 911412d..7b57616 100644 --- a/src/contracts/libs/StakingContractStorageLib.sol +++ b/src/contracts/libs/StakingContractStorageLib.sol @@ -315,21 +315,6 @@ library StakingContractStorageLib { =========================================== =========================================*/ - bytes32 internal constant WITHDRAWER_CUSTOMIZATION_ENABLED_SLOT = - keccak256("StakingContract.withdrawerCustomizationEnabled"); - - function getWithdrawerCustomizationEnabled() internal view returns (bool) { - return getBool(WITHDRAWER_CUSTOMIZATION_ENABLED_SLOT); - } - - function setWithdrawerCustomizationEnabled(bool _enabled) internal { - setBool(WITHDRAWER_CUSTOMIZATION_ENABLED_SLOT, _enabled); - } - - /* ======================================== - =========================================== - =========================================*/ - bytes32 internal constant EXIT_REQUEST_MAPPING_SLOT = bytes32(uint256(keccak256("StakingContract.exitRequest")) - 1); diff --git a/src/test/StakingContract.t.sol b/src/test/StakingContract.t.sol index ac13545..164127f 100644 --- a/src/test/StakingContract.t.sol +++ b/src/test/StakingContract.t.sol @@ -180,6 +180,7 @@ contract StakingContractTest is Test { assertEq(stakingContract.getAdmin(), admin); } + event BeginOwnershipTransfer(address indexed previousAdmin, address indexed newAdmin); event ChangedAdmin(address newAdmin); function testSetAdmin(address newAdmin) public { @@ -187,6 +188,8 @@ contract StakingContractTest is Test { // Start ownership transfer process. vm.startPrank(admin); + vm.expectEmit(true, true, true, true); + emit BeginOwnershipTransfer(admin, newAdmin); stakingContract.transferOwnership(newAdmin); vm.stopPrank(); // At this point, the old admin is still in charge. @@ -787,78 +790,6 @@ contract StakingContractTest is Test { vm.stopPrank(); } - event SetWithdrawerCustomizationStatus(bool _status); - - function testSetWithdrawer(address user, address anotherUser) public { - vm.assume(user != address(depositContract) && anotherUser != address(depositContract)); - vm.assume(anotherUser != address(0)); - vm.deal(user, 32 * 3 ether); - - vm.startPrank(user); - (bool _success, ) = address(stakingContract).call{value: 32 * 3 ether}(""); - assert(_success == true); - vm.stopPrank(); - - assertEq(user.balance, 0); - - bytes memory pk = PUBKEY_1; - - assertEq(stakingContract.getWithdrawer(pk), user); - - vm.startPrank(admin); - vm.expectEmit(true, true, true, true); - emit SetWithdrawerCustomizationStatus(true); - stakingContract.setWithdrawerCustomizationEnabled(true); - vm.stopPrank(); - - vm.startPrank(user); - stakingContract.setWithdrawer(pk, anotherUser); - vm.stopPrank(); - } - - function testSetWithdrawerForbidden(address user, address anotherUser) public { - vm.assume(user != address(depositContract) && anotherUser != address(depositContract)); - vm.deal(user, 32 * 3 ether); - - vm.startPrank(user); - (bool _success, ) = address(stakingContract).call{value: 32 * 3 ether}(""); - assert(_success == true); - vm.stopPrank(); - - assertEq(user.balance, 0); - - assertEq(stakingContract.getWithdrawer(PUBKEY_1), user); - - vm.startPrank(user); - vm.expectRevert(abi.encodeWithSignature("Forbidden()")); - stakingContract.setWithdrawer(PUBKEY_1, anotherUser); - vm.stopPrank(); - } - - function testSetWithdrawerUnauthorized(address user, address anotherUser) public { - vm.assume(user != address(depositContract) && anotherUser != address(depositContract)); - vm.assume(anotherUser != address(0) && anotherUser != user); - vm.deal(user, 32 * 3 ether); - - vm.startPrank(user); - (bool _success, ) = address(stakingContract).call{value: 32 * 3 ether}(""); - assert(_success == true); - vm.stopPrank(); - - assertEq(user.balance, 0); - - vm.startPrank(admin); - stakingContract.setWithdrawerCustomizationEnabled(true); - vm.stopPrank(); - - assertEq(stakingContract.getWithdrawer(PUBKEY_1), user); - - vm.startPrank(anotherUser); - vm.expectRevert(abi.encodeWithSignature("Unauthorized()")); - stakingContract.setWithdrawer(PUBKEY_1, anotherUser); - vm.stopPrank(); - } - event ChangedTreasury(address newTreasury); function testSetTreasury(address newTreasury) public { @@ -2055,6 +1986,18 @@ contract StakingContractBehindProxyTest is Test { assert(deactivated == false); } + event NewSanctionsOracle(address); + + function test_setSanctionsOracle() public { + assertEq(stakingContract.getSanctionsOracle(), address(0)); + vm.startPrank(admin); + vm.expectEmit(true, true, true, true); + emit NewSanctionsOracle(address(oracle)); + stakingContract.setSanctionsOracle(address(oracle)); + vm.stopPrank(); + assertEq(stakingContract.getSanctionsOracle(), address(oracle)); + } + function test_deposit_withsanctions_senderSanctioned(address user) public { assumeAddress(user); oracle.setSanction(user, true); @@ -3291,4 +3234,33 @@ contract StakingContractBehindProxyTest is Test { vm.prank(admin); stakingContract.blockAccount(bob, PUBKEY_2); } + + error AddressSanctioned(address addr); + + function test_withdraw_from_recipient_owner_sanctioned() public { + vm.prank(admin); + stakingContract.setSanctionsOracle(address(oracle)); + assertFalse(oracle.isSanctioned(bob)); + + vm.deal(bob, 32 ether); + + vm.prank(bob); + stakingContract.deposit{value: 32 ether}(); + + address clfr = stakingContract.getCLFeeRecipient(PUBKEY_1); + vm.deal(clfr, 1 ether); + + vm.prank(bob); + // First withdrawal deploy the recipient such that afterwards it's possible to trigger + // the withdrawal from the recipient directly + stakingContract.withdrawCLFee(PUBKEY_1); + + oracle.setSanction(bob, true); + + vm.deal(clfr, 1 ether); + + vm.prank(bob); + vm.expectRevert(abi.encodeWithSignature("AddressSanctioned(address)", bob)); + IFeeRecipient(clfr).withdraw(); + } }