Fixes and improvements from SP audit and devnet testing

Author: kanewallmann

contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsMinipool.sol

@@ -99,6 +99,13 @@ contract RocketDAOProtocolSettingsMinipool is RocketDAOProtocolSettings, RocketD
         return (_time >= start && _time < (start.add(length)));
     }
 
+    /// @notice Returns true if the given time has passed the distribute window
+    function hasUserDistributeWindowPassed(uint256 _time) override external view returns (bool) {
+        uint256 start = getUserDistributeWindowStart();
+        uint256 length = getUserDistributeWindowLength();
+        return _time >= start.add(length);
+    }
+
     /// @notice Returns the start of the user distribute window
     function getUserDistributeWindowStart() override public view returns (uint256) {
         return getSettingUint("minipool.user.distribute.window.start");

contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsNode.sol

@@ -12,13 +12,14 @@ contract RocketDAOProtocolSettingsNode is RocketDAOProtocolSettings, RocketDAOPr
     // Construct
     constructor(RocketStorageInterface _rocketStorageAddress) RocketDAOProtocolSettings(_rocketStorageAddress, "node") {
         // Set version
-        version = 2;
+        version = 3;
         // Initialize settings on deployment
         if(!getBool(keccak256(abi.encodePacked(settingNameSpace, "deployed")))) {
             // Apply settings
             setSettingBool("node.registration.enabled", false);
             setSettingBool("node.smoothing.pool.registration.enabled", true);
             setSettingBool("node.deposit.enabled", false);
+            setSettingBool("node.vacant.minipools.enabled", false);
             setSettingUint("node.per.minipool.stake.minimum", 0.1 ether);      // 10% of user ETH value
             setSettingUint("node.per.minipool.stake.maximum", 1.5 ether);      // 150% of user ETH value
             // Settings initialised
@@ -41,6 +42,11 @@ contract RocketDAOProtocolSettingsNode is RocketDAOProtocolSettings, RocketDAOPr
         return getSettingBool("node.deposit.enabled");
     }
 
+    // Vacant minipools currently enabled
+    function getVacantMinipoolsEnabled() override external view returns (bool) {
+        return getSettingBool("node.vacant.minipools.enabled");
+    }
+
     // Minimum RPL stake per minipool as a fraction of assigned user ETH value
     function getMinimumPerMinipoolStake() override external view returns (uint256) {
         return getSettingUint("node.per.minipool.stake.minimum");

contracts/contract/deposit/RocketDepositPool.sol

@@ -148,11 +148,14 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul
     }
 
     /// @dev Accepts ETH deposit from the node deposit contract (does not mint rETH)
-    function nodeDeposit() override external payable onlyThisLatestContract onlyLatestContract("rocketNodeDeposit", msg.sender) {
+    /// @param _totalAmount The total node deposit amount including any credit balance used
+    function nodeDeposit(uint256 _totalAmount) override external payable onlyThisLatestContract onlyLatestContract("rocketNodeDeposit", msg.sender) {
         // Deposit ETH into the vault
-        rocketVault.depositEther{value: msg.value}();
+        if (msg.value > 0) {
+            rocketVault.depositEther{value: msg.value}();
+        }
         // Increase recorded node balance
-        addUint("deposit.pool.node.balance", msg.value);
+        addUint("deposit.pool.node.balance", _totalAmount);
     }
 
     /// @dev Withdraws ETH from the deposit pool to RocketNodeDeposit contract to be used for a new minipool

contracts/contract/minipool/RocketMinipoolBase.sol

@@ -14,6 +14,19 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
     event DelegateUpgraded(address oldDelegate, address newDelegate, uint256 time);
     event DelegateRolledBack(address oldDelegate, address newDelegate, uint256 time);
 
+    // Store a reference to the address of RocketMinipoolBase itself to prevent direct calls to this contract
+    address immutable self;
+
+    constructor () {
+        self = address(this);
+    }
+
+    /// @dev Prevent direct calls to this contract
+    modifier notSelf() {
+        require(address(this) != self);
+        _;
+    }
+
     /// @dev Only allow access from the owning node address
     modifier onlyMinipoolOwner() {
         // Only the node operator can upgrade
@@ -23,9 +36,10 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
     }
 
     /// @notice Sets up starting delegate contract and then delegates initialisation to it
-    function initialise(address _nodeAddress) external {
+    function initialise(address _nodeAddress) external notSelf {
         // Check input
         require(_nodeAddress != address(0), "Invalid node address");
+        require(storageState == StorageState.Undefined, "Already initialised");
         // Set storage state to uninitialised
         storageState = StorageState.Uninitialised;
         // Set the current delegate
@@ -39,13 +53,13 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
     }
 
     /// @notice Receive an ETH deposit
-    receive() external payable {
+    receive() external payable notSelf {
         // Emit ether received event
         emit EtherReceived(msg.sender, msg.value, block.timestamp);
     }
 
     /// @notice Upgrade this minipool to the latest network delegate contract
-    function delegateUpgrade() external onlyMinipoolOwner {
+    function delegateUpgrade() external onlyMinipoolOwner notSelf {
         // Set previous address
         rocketMinipoolDelegatePrev = rocketMinipoolDelegate;
         // Set new delegate
@@ -57,7 +71,7 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
     }
 
     /// @notice Rollback to previous delegate contract
-    function delegateRollback() external onlyMinipoolOwner {
+    function delegateRollback() external onlyMinipoolOwner notSelf {
         // Make sure they have upgraded before
         require(rocketMinipoolDelegatePrev != address(0x0), "Previous delegate contract is not set");
         // Store original
@@ -71,7 +85,7 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
 
     /// @notice Sets the flag to automatically use the latest delegate contract or not
     /// @param _setting If true, will always use the latest delegate contract
-    function setUseLatestDelegate(bool _setting) external onlyMinipoolOwner {
+    function setUseLatestDelegate(bool _setting) external onlyMinipoolOwner notSelf {
         useLatestDelegate = _setting;
     }
 
@@ -96,7 +110,7 @@ contract RocketMinipoolBase is RocketMinipoolStorageLayout {
     }
 
     /// @notice Delegates all calls to minipool delegate contract (or latest if flag is set)
-    fallback(bytes calldata _input) external payable returns (bytes memory) {
+    fallback(bytes calldata _input) external payable notSelf returns (bytes memory) {
         // If useLatestDelegate is set, use the latest delegate contract
         address delegateContract = useLatestDelegate ? getContractAddress("rocketMinipoolDelegate") : rocketMinipoolDelegate;
         // Check for contract existence

contracts/contract/minipool/RocketMinipoolBondReducer.sol

@@ -5,8 +5,11 @@ pragma abicoder v2;
 import "@openzeppelin/contracts/math/SafeMath.sol";
 
 import "../RocketBase.sol";
-import "./RocketMinipoolDelegate.sol";
+import "../../interface/minipool/RocketMinipoolInterface.sol";
 import "../../interface/minipool/RocketMinipoolBondReducerInterface.sol";
+import "../../interface/node/RocketNodeDepositInterface.sol";
+import "../../interface/dao/node/settings/RocketDAONodeTrustedSettingsMinipoolInterface.sol";
+import "../../interface/dao/node/RocketDAONodeTrustedInterface.sol";
 
 /// @notice Handles bond reduction window and trusted node cancellation
 contract RocketMinipoolBondReducer is RocketBase, RocketMinipoolBondReducerInterface {
@@ -15,7 +18,7 @@ contract RocketMinipoolBondReducer is RocketBase, RocketMinipoolBondReducerInter
     using SafeMath for uint;
 
     // Events
-    event BeginBondReduction(address indexed minipool, uint256 time);
+    event BeginBondReduction(address indexed minipool, uint256 newBondAmount, uint256 time);
     event CancelReductionVoted(address indexed minipool, address indexed member, uint256 time);
     event ReductionCancelled(address indexed minipool, uint256 time);
 
@@ -26,21 +29,47 @@ contract RocketMinipoolBondReducer is RocketBase, RocketMinipoolBondReducerInter
     /// @notice Flags a minipool as wanting to reduce collateral, owner can then call `reduceBondAmount` once waiting
     ///         period has elapsed
     /// @param _minipoolAddress Address of the minipool
-    function beginReduceBondAmount(address _minipoolAddress) override external onlyLatestContract("rocketMinipoolBondReducer", address(this)) {
-        RocketMinipoolDelegate minipool = RocketMinipoolDelegate(_minipoolAddress);
+    /// @param _newBondAmount The new bond amount
+    function beginReduceBondAmount(address _minipoolAddress, uint256 _newBondAmount) override external onlyLatestContract("rocketMinipoolBondReducer", address(this)) {
+        RocketMinipoolInterface minipool = RocketMinipoolInterface(_minipoolAddress);
+        RocketNodeDepositInterface rocketNodeDeposit = RocketNodeDepositInterface(getContractAddress("rocketNodeDeposit"));
         require(msg.sender == minipool.getNodeAddress(), "Only minipool owner");
         // Check if has been previously cancelled
         bool reductionCancelled = getBool(keccak256(abi.encodePacked("minipool.bond.reduction.cancelled", address(minipool))));
         require(!reductionCancelled, "This minipool is not allowed to reduce bond");
         require(minipool.getStatus() == MinipoolStatus.Staking, "Minipool must be staking");
+        // Check if new bond amount is valid
+        require(rocketNodeDeposit.isValidDepositAmount(_newBondAmount), "Invalid bond amount");
+        uint256 existing = minipool.getNodeDepositBalance();
+        require(_newBondAmount < existing, "Bond must be lower than current amount");
+        // Store time and new bond amount
         setUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", _minipoolAddress)), block.timestamp);
-        emit BeginBondReduction(_minipoolAddress, block.timestamp);
+        setUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", _minipoolAddress)), _newBondAmount);
+        emit BeginBondReduction(_minipoolAddress, _newBondAmount, block.timestamp);
+    }
+
+    /// @notice Returns the timestamp of when a given minipool began their bond reduction waiting period
+    /// @param _minipoolAddress Address of the minipool to query
+    function getReduceBondTime(address _minipoolAddress) override external view returns (uint256) {
+        return getUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", _minipoolAddress)));
+    }
+
+    /// @notice Returns the new bond that a given minipool has indicated they are reducing to
+    /// @param _minipoolAddress Address of the minipool to query
+    function getReduceBondValue(address _minipoolAddress) override external view returns (uint256) {
+        return getUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", _minipoolAddress)));
+    }
+
+    /// @notice Returns true if the given minipool has had it's bond reduction cancelled by the oDAO
+    /// @param _minipoolAddress Address of the minipool to query
+    function getReduceBondCancelled(address _minipoolAddress) override public view returns (bool) {
+        return getBool(keccak256(abi.encodePacked("minipool.bond.reduction.cancelled", address(_minipoolAddress))));
     }
 
     /// @notice Returns whether owner of given minipool can reduce bond amount given the waiting period constraint
     /// @param _minipoolAddress Address of the minipool
     function canReduceBondAmount(address _minipoolAddress) override public view returns (bool) {
-        RocketMinipoolDelegate minipool = RocketMinipoolDelegate(_minipoolAddress);
+        RocketMinipoolInterface minipool = RocketMinipoolInterface(_minipoolAddress);
         RocketDAONodeTrustedSettingsMinipoolInterface rocketDAONodeTrustedSettingsMinipool = RocketDAONodeTrustedSettingsMinipoolInterface(getContractAddress("rocketDAONodeTrustedSettingsMinipool"));
         uint256 reduceBondTime = getUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", _minipoolAddress)));
         return rocketDAONodeTrustedSettingsMinipool.isWithinBondReductionWindow(block.timestamp.sub(reduceBondTime));
@@ -49,8 +78,10 @@ contract RocketMinipoolBondReducer is RocketBase, RocketMinipoolBondReducerInter
     /// @notice Can be called by trusted nodes to cancel a reduction in bond if the validator has too low of a balance
     /// @param _minipoolAddress Address of the minipool
     function voteCancelReduction(address _minipoolAddress) override external onlyTrustedNode(msg.sender) onlyLatestContract("rocketMinipoolBondReducer", address(this)) {
-        RocketMinipoolDelegate minipool = RocketMinipoolDelegate(_minipoolAddress);
+        // Prevent calling if consensus has already been reached
+        require(!getReduceBondCancelled(_minipoolAddress), "Already cancelled");
         // Get contracts
+        RocketMinipoolInterface minipool = RocketMinipoolInterface(_minipoolAddress);
         RocketDAONodeTrustedInterface rocketDAONode = RocketDAONodeTrustedInterface(getContractAddress("rocketDAONodeTrusted"));
         // Check for multiple votes
         bytes32 memberVotedKey = keccak256(abi.encodePacked("minipool.bond.reduction.member.voted", _minipoolAddress, msg.sender));
@@ -69,33 +100,39 @@ contract RocketMinipoolBondReducer is RocketBase, RocketMinipoolBondReducerInter
             emit ReductionCancelled(_minipoolAddress, block.timestamp);
             setBool(keccak256(abi.encodePacked("minipool.bond.reduction.cancelled", _minipoolAddress)), true);
             deleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", _minipoolAddress)));
+            deleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", msg.sender)));
         } else {
             // Increment total
             setUint(totalCancelVotesKey, totalCancelVotes);
         }
     }
 
     /// @notice Called by minipools when they are reducing bond to handle state changes outside the minipool
-    /// @param _from The previous bond amount
-    /// @param _to The new bond amount
-    function reduceBondAmount(uint256 _from, uint256 _to) override external onlyRegisteredMinipool(msg.sender) onlyLatestContract("rocketMinipoolBondReducer", address(this)) {
+    function reduceBondAmount() override external onlyRegisteredMinipool(msg.sender) onlyLatestContract("rocketMinipoolBondReducer", address(this)) returns (uint256) {
+        // Get contracts
+        RocketNodeDepositInterface rocketNodeDeposit = RocketNodeDepositInterface(getContractAddress("rocketNodeDeposit"));
+        RocketMinipoolInterface minipool = RocketMinipoolInterface(msg.sender);
         // Check if has been cancelled
         bool reductionCancelled = getBool(keccak256(abi.encodePacked("minipool.bond.reduction.cancelled", address(msg.sender))));
         require(!reductionCancelled, "This minipool is not allowed to reduce bond");
         // Check wait period is satisfied
         require(canReduceBondAmount(msg.sender), "Wait period not satisfied");
-        // Get contracts
-        RocketNodeDepositInterface rocketNodeDeposit = RocketNodeDepositInterface(getContractAddress("rocketNodeDeposit"));
-        RocketMinipoolDelegate minipool = RocketMinipoolDelegate(msg.sender);
-        // Check the new bond amount is valid
-        require(rocketNodeDeposit.isValidDepositAmount(_to), "Invalid bond amount");
+        // Get desired to amount
+        uint256 newBondAmount = getUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", msg.sender)));
+        require(rocketNodeDeposit.isValidDepositAmount(newBondAmount), "Invalid bond amount");
         // Calculate difference
-        uint256 delta = _from.sub(_to);
+        uint256 existingBondAmount = minipool.getNodeDepositBalance();
+        uint256 delta = existingBondAmount.sub(newBondAmount);
         // Get node address
         address nodeAddress = minipool.getNodeAddress();
         // Increase ETH matched or revert if exceeds limit based on current RPL stake
         rocketNodeDeposit.increaseEthMatched(nodeAddress, delta);
         // Increase node operator's deposit credit
         rocketNodeDeposit.increaseDepositCreditBalance(nodeAddress, delta);
+        // Clean up state
+        deleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", msg.sender)));
+        deleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", msg.sender)));
+        // Return
+        return newBondAmount;
     }
 }