Skip to content

Commit

Permalink
Fixes and increased test coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
kanewallmann committed Nov 18, 2022
1 parent 0eb320a commit 602ed4a
Show file tree
Hide file tree
Showing 13 changed files with 310 additions and 34 deletions.
18 changes: 11 additions & 7 deletions contracts/contract/minipool/RocketMinipoolDelegate.sol
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,10 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
RocketMinipoolManagerInterface rocketMinipoolManager = RocketMinipoolManagerInterface(getContractAddress("rocketMinipoolManager"));
require(rocketMinipoolManager.getMinipoolExists(address(this)), "Minipool already closed");
rocketMinipoolManager.destroyMinipool();
// Clear state
userDepositBalance = 0;
userDepositBalanceLegacy = 0;
userDepositAssignedTime = 0;
}

/// @notice Can be called by trusted nodes to scrub this minipool if its withdrawal credentials are not set correctly
Expand Down Expand Up @@ -637,20 +641,23 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
require(_amount < previousBond, "Bond must be lower than current amount");
require(status == MinipoolStatus.Staking, "Minipool must be staking");
// Get contracts
RocketNodeDepositInterface rocketNodeDepositInterface = RocketNodeDepositInterface(getContractAddress("rocketNodeDeposit"));
RocketNodeDepositInterface rocketNodeDeposit = RocketNodeDepositInterface(getContractAddress("rocketNodeDeposit"));
// Check the new bond amount is valid
require(rocketNodeDepositInterface.isValidDepositAmount(_amount), "Invalid bond amount");
require(rocketNodeDeposit.isValidDepositAmount(_amount), "Invalid bond amount");
// Distribute any skimmed rewards
distributeSkimmedRewards();
// Update user/node balances
// Calculate bond difference
uint256 delta = previousBond.sub(_amount);
// Increase ETH matched or revert if exceeds limit based on current RPL stake
rocketNodeDeposit.increaseEthMatched(nodeAddress, delta);
// Update user/node balances
userDepositBalance = getUserDepositBalance().add(delta);
nodeDepositBalance = _amount;
// Reset node fee to current network rate
RocketNetworkFeesInterface rocketNetworkFees = RocketNetworkFeesInterface(getContractAddress("rocketNetworkFees"));
nodeFee = rocketNetworkFees.getNodeFee();
// Increase node operator's deposit credit
rocketNodeDepositInterface.increaseDepositCreditBalance(nodeAddress, delta);
rocketNodeDeposit.increaseDepositCreditBalance(nodeAddress, delta);
// Break state to prevent rollback exploit
if (depositType != MinipoolDeposit.Variable) {
userDepositBalanceLegacy = 2**256-1;
Expand Down Expand Up @@ -724,9 +731,6 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
emit EtherWithdrawn(address(rocketDepositPool), userCapital, block.timestamp);
}
// Clear storage
userDepositBalance = 0;
userDepositBalanceLegacy = 0;
userDepositAssignedTime = 0;
nodeDepositBalance = 0;
nodeRefundBalance = 0;
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/contract/minipool/RocketMinipoolFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract RocketMinipoolFactory is RocketBase, RocketMinipoolFactoryInterface {
function deployContract(address _nodeAddress, uint256 _salt) override external onlyLatestContract("rocketMinipoolFactory", address(this)) onlyLatestContract("rocketMinipoolManager", msg.sender) returns (address) {
// Construct deployment bytecode
bytes memory creationCode = getMinipoolBytecode();
bytes memory bytecode = abi.encodePacked(creationCode, abi.encode(rocketStorage, _nodeAddress));
bytes memory bytecode = abi.encodePacked(creationCode, abi.encode(rocketStorage));
// Construct final salt
uint256 salt = uint256(keccak256(abi.encodePacked(_nodeAddress, _salt)));
// CREATE2 deployment
Expand Down
14 changes: 11 additions & 3 deletions contracts/contract/node/RocketNodeDeposit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ contract RocketNodeDeposit is RocketBase, RocketNodeDepositInterface {
RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool"));
launchAmount = rocketDAOProtocolSettingsMinipool.getLaunchBalance();
}
increaseEthMatched(msg.sender, launchAmount.sub(_bondAmount));
_increaseEthMatched(msg.sender, launchAmount.sub(_bondAmount));
// Create the minipool
RocketMinipoolInterface minipool = createMinipool(_salt, _expectedMinipoolAddress);
// Get the pre-launch value
Expand Down Expand Up @@ -140,14 +140,22 @@ contract RocketNodeDeposit is RocketBase, RocketNodeDepositInterface {
// Increase ETH matched (used to calculate RPL collateral requirements)
RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool"));
uint256 launchAmount = rocketDAOProtocolSettingsMinipool.getLaunchBalance();
increaseEthMatched(msg.sender, launchAmount.sub(_bondAmount));
_increaseEthMatched(msg.sender, launchAmount.sub(_bondAmount));
// Create the minipool
_createVacantMinipool(_salt, _validatorPubkey, _bondAmount, _expectedMinipoolAddress);
}

/// @notice Called by minipools during bond reduction to increase the amount of ETH the node operator has
/// @param _nodeAddress The node operator's address to increase the ETH matched for
/// @param _amount The amount to increase the ETH matched
/// @dev Will revert if the new ETH matched amount exceeds the node operators limit
function increaseEthMatched(address _nodeAddress, uint256 _amount) override external onlyLatestContract("rocketNodeDeposit", address(this)) onlyRegisteredMinipool(msg.sender) {
_increaseEthMatched(_nodeAddress, _amount);
}

/// @dev Increases the amount of ETH that has been matched against a node operators bond. Reverts if it exceeds the
/// collateralisation requirements of the network
function increaseEthMatched(address _nodeAddress, uint256 _amount) private {
function _increaseEthMatched(address _nodeAddress, uint256 _amount) private {
// Check amount doesn't exceed limits
RocketNodeStakingInterface rocketNodeStaking = RocketNodeStakingInterface(getContractAddress("rocketNodeStaking"));
require(
Expand Down
17 changes: 17 additions & 0 deletions contracts/contract/upgrade/RocketUpgradeOneDotTwo.sol
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ contract RocketUpgradeOneDotTwo is RocketBase {
string public newRocketNodeManagerAbi;
string public rocketMinipoolBaseAbi;

string public newRocketMinipoolAbi;

// Save deployer to limit access to set functions
address immutable deployer;

Expand Down Expand Up @@ -115,6 +117,7 @@ contract RocketUpgradeOneDotTwo is RocketBase {
newRocketDAONodeTrustedSettingsMinipoolAbi = _abis[12];
newRocketNodeManagerAbi = _abis[13];
rocketMinipoolBaseAbi = _abis[14];
newRocketMinipoolAbi = _abis[15];
}

function setInterval(uint256 _interval, uint256 _block) external {
Expand Down Expand Up @@ -156,6 +159,9 @@ contract RocketUpgradeOneDotTwo is RocketBase {
// Add new contracts
_addContract("rocketMinipoolBase", rocketMinipoolBase, rocketMinipoolBaseAbi);

// Upgrade ABIs
_upgradeABI("rocketMinipool", newRocketMinipoolAbi);

// Migrate settings
bytes32 settingNameSpace = keccak256(abi.encodePacked("dao.protocol.setting.", "deposit"));
setUint(keccak256(abi.encodePacked(settingNameSpace, "deposit.assign.maximum")), 90);
Expand Down Expand Up @@ -241,4 +247,15 @@ contract RocketUpgradeOneDotTwo is RocketBase {
deleteString(keccak256(abi.encodePacked("contract.abi", _name)));
}

/// @dev Upgrade a network contract ABI
function _upgradeABI(string memory _name, string memory _contractAbi) internal {
// Check ABI exists
string memory existingAbi = getString(keccak256(abi.encodePacked("contract.abi", _name)));
require(bytes(existingAbi).length > 0, "ABI does not exist");
// Sanity checks
require(bytes(_contractAbi).length > 0, "Empty ABI is invalid");
require(keccak256(bytes(existingAbi)) != keccak256(bytes(_contractAbi)), "ABIs are identical");
// Set ABI
setString(keccak256(abi.encodePacked("contract.abi", _name)), _contractAbi);
}
}
1 change: 1 addition & 0 deletions contracts/interface/node/RocketNodeDepositInterface.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ interface RocketNodeDepositInterface {
function isValidDepositAmount(uint256 _amount) external pure returns (bool);
function getDepositAmounts() external pure returns (uint256[] memory);
function createVacantMinipool(uint256 _bondAmount, uint256 _minimumNodeFee, bytes calldata _validatorPubkey, uint256 _salt, address _expectedMinipoolAddress) external;
function increaseEthMatched(address _nodeAddress, uint256 _amount) external;
}
11 changes: 11 additions & 0 deletions scripts/deploy-upgrade-v1.2.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,16 @@ const contracts = {
rocketUpgradeOneDotTwo: artifacts.require('RocketUpgradeOneDotTwo.sol'),
};

// Construct ABI for rocketMinipool
const rocketMinipoolAbi = []
.concat(artifacts.require('RocketMinipoolDelegate.sol').abi)
.concat(artifacts.require('RocketMinipoolBase.sol').abi)
.concat(artifacts.require('RocketMinipoolProxy.sol').abi)
.filter(i => i.type !== 'fallback' && i.type !== 'receive');

rocketMinipoolAbi.push({ stateMutability: 'payable', type: 'fallback'});
rocketMinipoolAbi.push({ stateMutability: 'payable', type: 'receive'});

/*** Deployment **********************/

// Upgrade Rocket Pool
Expand Down Expand Up @@ -117,6 +127,7 @@ export async function upgrade() {
compressABI(contracts.rocketDAONodeTrustedSettingsMinipool.abi),
compressABI(contracts.rocketNodeManager.abi),
compressABI(contracts.rocketMinipoolBase.abi),
rocketMinipoolAbi
],
];
await instance.set(...args);
Expand Down
10 changes: 10 additions & 0 deletions test/_helpers/deployment.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ const abis = {
rocketMinipool: [artifacts.require('RocketMinipoolDelegateOld.sol'), artifacts.require('RocketMinipoolOld.sol')],
};

// Construct ABI for rocketMinipool
const rocketMinipoolAbi = []
.concat(artifacts.require('RocketMinipoolDelegate.sol').abi)
.concat(artifacts.require('RocketMinipoolBase.sol').abi)
.concat(artifacts.require('RocketMinipoolProxy.sol').abi)
.filter(i => i.type !== 'fallback' && i.type !== 'receive');

rocketMinipoolAbi.push({ stateMutability: 'payable', type: 'fallback'});
rocketMinipoolAbi.push({ stateMutability: 'payable', type: 'receive'});

/*** Deployment **********************/

Expand Down Expand Up @@ -266,6 +275,7 @@ export async function deployRocketPool() {
compressABI(contracts.rocketDAONodeTrustedSettingsMinipoolNew.abi),
compressABI(contracts.rocketNodeManagerNew.abi),
compressABI(contracts.rocketMinipoolBase.abi),
compressABI(rocketMinipoolAbi),
],
]
await upgrader.set(...args)
Expand Down
10 changes: 7 additions & 3 deletions test/_helpers/minipool.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ let minipoolSalt = 1

// Create a minipool
export async function createMinipool(txOptions, salt = null) {
return createMinipoolWithBondAmount(txOptions.value, txOptions, salt);
}

export async function createMinipoolWithBondAmount(bond, txOptions, salt = null) {
const preUpdate = !(await upgradeExecuted());

// Load contracts
Expand Down Expand Up @@ -101,7 +105,7 @@ export async function createMinipool(txOptions, salt = null) {
const depositType = await rocketNodeDeposit.getDepositType(txOptions.value);
constructorArgs = web3.eth.abi.encodeParameters(['address', 'address', 'uint8'], [rocketStorage.address, txOptions.from, depositType]);
} else {
constructorArgs = web3.eth.abi.encodeParameters(['address', 'address'], [rocketStorage.address, txOptions.from]);
constructorArgs = web3.eth.abi.encodeParameters(['address'], [rocketStorage.address]);
}
const deployCode = contractBytecode + constructorArgs.substr(2);

Expand Down Expand Up @@ -155,7 +159,7 @@ export async function createMinipool(txOptions, salt = null) {

let depositDataRoot = getDepositDataRoot(depositData);

await rocketNodeDeposit.deposit(txOptions.value, '0'.ether, depositData.pubkey, depositData.signature, depositDataRoot, salt, '0x' + minipoolAddress, txOptions);
await rocketNodeDeposit.deposit(bond, '0'.ether, depositData.pubkey, depositData.signature, depositDataRoot, salt, '0x' + minipoolAddress, txOptions);
}

return RocketMinipoolDelegate.at('0x' + minipoolAddress);
Expand All @@ -180,7 +184,7 @@ export async function createVancantMinipool(bondAmount, txOptions, salt = null)
const contractBytecode = RocketMinipoolProxy.bytecode;

// Construct creation code for minipool deploy
const constructorArgs = web3.eth.abi.encodeParameters(['address', 'address'], [rocketStorage.address, txOptions.from]);
const constructorArgs = web3.eth.abi.encodeParameters(['address'], [rocketStorage.address]);
const deployCode = contractBytecode + constructorArgs.substr(2);

if (salt === null){
Expand Down
2 changes: 1 addition & 1 deletion test/_helpers/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ export async function nodeDeposit(txOptions) {
const contractBytecode = RocketMinipoolProxy.bytecode;

// Construct creation code for minipool deploy
const constructorArgs = web3.eth.abi.encodeParameters(['address', 'address'], [rocketStorage.address, txOptions.from]);
const constructorArgs = web3.eth.abi.encodeParameters(['address'], [rocketStorage.address]);
const deployCode = contractBytecode + constructorArgs.substr(2);
const salt = minipoolSalt++;

Expand Down
1 change: 0 additions & 1 deletion test/minipool/scenario-dissolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ export async function dissolve(minipool, txOptions) {
// Check minipool details
assertBN.notEqual(details1.status, minipoolStates.Dissolved, 'Incorrect initial minipool status');
assertBN.equal(details2.status, minipoolStates.Dissolved, 'Incorrect updated minipool status');
assertBN.equal(details2.userDepositBalance, 0, 'Incorrect updated minipool user deposit balance');
}
1 change: 0 additions & 1 deletion test/minipool/scenario-scrub.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export async function voteScrub(minipool, txOptions) {
// Check state
if (details1.votes.add('1'.BN).gt(quorum)){
assertBN.equal(details2.status, minipoolStates.Dissolved, 'Incorrect updated minipool status');
assertBN.equal(details2.userDepositBalance, '0', 'Incorrect updated minipool user deposit balance');
// Check slashing if penalties are enabled
if (details1.penaltyEnabled && !details1.vacant) {
// Calculate amount slashed
Expand Down
2 changes: 1 addition & 1 deletion test/node/scenario-deposit-v2.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export async function depositV2(minimumNodeFee, bondAmount, txOptions) {
const contractBytecode = RocketMinipoolProxy.bytecode;

// Construct creation code for minipool deploy
const constructorArgs = web3.eth.abi.encodeParameters(['address', 'address'], [rocketStorage.address, txOptions.from]);
const constructorArgs = web3.eth.abi.encodeParameters(['address'], [rocketStorage.address]);
const deployCode = contractBytecode + constructorArgs.substr(2);
const salt = minipoolSalt++;

Expand Down
Loading

0 comments on commit 602ed4a

Please sign in to comment.