Multiple checks-effects violations
Throughout the system, there are various violations of the checks-effects-interactions pattern where the contract state is updated after an external call. Since large parts of the Rocket Pool system's smart contracts are not guarded against reentrancy, the external call's recipient may reenter and potentially perform malicious actions that can impact the overall accounting and, thus, system funds.\\n`distributeToOwner()` sends the contract's balance to the node or the withdrawal address before clearing the internal accounting:\\n```\\n/// @notice Withdraw node balances from the minipool and close it. Only accepts calls from the owner\\nfunction close() override external onlyMinipoolOwner(msg.sender) onlyInitialised {\\n    // Check current status\\n    require(status == MinipoolStatus.Dissolved, "The minipool can only be closed while dissolved");\\n    // Distribute funds to owner\\n    distributeToOwner();\\n    // Destroy minipool\\n    RocketMinipoolManagerInterface rocketMinipoolManager = RocketMinipoolManagerInterface(getContractAddress("rocketMinipoolManager"));\\n    require(rocketMinipoolManager.getMinipoolExists(address(this)), "Minipool already closed");\\n    rocketMinipoolManager.destroyMinipool();\\n    // Clear state\\n    nodeDepositBalance = 0;\\n    nodeRefundBalance = 0;\\n    userDepositBalance = 0;\\n    userDepositBalanceLegacy = 0;\\n    userDepositAssignedTime = 0;\\n}\\n```\\n\\nThe withdrawal block should be set before any other contracts are called:\\n```\\n// Save block to prevent multiple withdrawals within a few blocks\\nwithdrawalBlock = block.number;\\n```\\n\\nThe `slashed` state should be set before any external calls are made:\\n```\\n/// @dev Slash node operator's RPL balance based on nodeSlashBalance\\nfunction \\_slash() private {\\n    // Get contracts\\n    RocketNodeStakingInterface rocketNodeStaking = RocketNodeStakingInterface(getContractAddress("rocketNodeStaking"));\\n    // Slash required amount and reset storage value\\n    uint256 slashAmount = nodeSlashBalance;\\n    nodeSlashBalance = 0;\\n    rocketNodeStaking.slashRPL(nodeAddress, slashAmount);\\n    // Record slashing\\n    slashed = true;\\n}\\n```\\n\\nIn the bond reducer, the accounting values should be cleared before any external calls are made:\\n```\\n// Get desired to amount\\nuint256 newBondAmount = getUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", msg.sender)));\\nrequire(rocketNodeDeposit.isValidDepositAmount(newBondAmount), "Invalid bond amount");\\n// Calculate difference\\nuint256 existingBondAmount = minipool.getNodeDepositBalance();\\nuint256 delta = existingBondAmount.sub(newBondAmount);\\n// Get node address\\naddress nodeAddress = minipool.getNodeAddress();\\n// Increase ETH matched or revert if exceeds limit based on current RPL stake\\nrocketNodeDeposit.increaseEthMatched(nodeAddress, delta);\\n// Increase node operator's deposit credit\\nrocketNodeDeposit.increaseDepositCreditBalance(nodeAddress, delta);\\n// Clean up state\\ndeleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.time", msg.sender)));\\ndeleteUint(keccak256(abi.encodePacked("minipool.bond.reduction.value", msg.sender)));\\n```\\n\\nThe counter for reward snapshot execution should be incremented before RPL gets minted:\\n```\\n// Execute inflation if required\\nrplContract.inflationMintTokens();\\n// Increment the reward index and update the claim interval timestamp\\nincrementRewardIndex();\\n```\\n
We recommend following the checks-effects-interactions pattern and adjusting any contract state variables before making external calls. With the upgradeable nature of the system, we also recommend strictly adhering to this practice when all external calls are being made to trusted network contracts.
```\\n/// @notice Withdraw node balances from the minipool and close it. Only accepts calls from the owner\\nfunction close() override external onlyMinipoolOwner(msg.sender) onlyInitialised {\\n    // Check current status\\n    require(status == MinipoolStatus.Dissolved, "The minipool can only be closed while dissolved");\\n    // Distribute funds to owner\\n    distributeToOwner();\\n    // Destroy minipool\\n    RocketMinipoolManagerInterface rocketMinipoolManager = RocketMinipoolManagerInterface(getContractAddress("rocketMinipoolManager"));\\n    require(rocketMinipoolManager.getMinipoolExists(address(this)), "Minipool already closed");\\n    rocketMinipoolManager.destroyMinipool();\\n    // Clear state\\n    nodeDepositBalance = 0;\\n    nodeRefundBalance = 0;\\n    userDepositBalance = 0;\\n    userDepositBalanceLegacy = 0;\\n    userDepositAssignedTime = 0;\\n}\\n```\\n