Skip to content

Commit ec3c5fa

Browse files
kanewallmannkanewallmann
committed
Prevent vacant minipool from refunding
1 parent 8b995d0 commit ec3c5fa

2 files changed

Lines changed: 63 additions & 32 deletions

File tree

‎contracts/contract/minipool/RocketMinipoolDelegate.sol‎

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -47,26 +47,39 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
4747
event EtherWithdrawalProcessed(address indexed executed, uint256 nodeAmount, uint256 userAmount, uint256 totalBalance, uint256 time);
4848

4949
// Status getters
50-
function getStatus() override external view returns (MinipoolStatus) { return status; }
51-
function getFinalised() override external view returns (bool) { return finalised; }
52-
function getStatusBlock() override external view returns (uint256) { return statusBlock; }
53-
function getStatusTime() override external view returns (uint256) { return statusTime; }
54-
function getScrubVoted(address _member) override external view returns (bool) { return memberScrubVotes[_member]; }
50+
function getStatus() override external view returns (MinipoolStatus) {return status;}
51+
52+
function getFinalised() override external view returns (bool) {return finalised;}
53+
54+
function getStatusBlock() override external view returns (uint256) {return statusBlock;}
55+
56+
function getStatusTime() override external view returns (uint256) {return statusTime;}
57+
58+
function getScrubVoted(address _member) override external view returns (bool) {return memberScrubVotes[_member];}
5559

5660
// Deposit type getter
57-
function getDepositType() override external view returns (MinipoolDeposit) { return depositType; }
61+
function getDepositType() override external view returns (MinipoolDeposit) {return depositType;}
5862

5963
// Node detail getters
60-
function getNodeAddress() override external view returns (address) { return nodeAddress; }
61-
function getNodeFee() override external view returns (uint256) { return nodeFee; }
62-
function getNodeDepositBalance() override external view returns (uint256) { return nodeDepositBalance; }
63-
function getNodeRefundBalance() override external view returns (uint256) { return nodeRefundBalance; }
64-
function getNodeDepositAssigned() override external view returns (bool) { return userDepositAssignedTime != 0; }
65-
function getPreLaunchValue() override external view returns (uint256) { return preLaunchValue; }
66-
function getNodeTopUpValue() override external view returns (uint256) { return nodeDepositBalance.sub(preLaunchValue); }
67-
function getVacant() override external view returns (bool) { return vacant; }
68-
function getPreMigrationBalance() override external view returns (uint256) { return preMigrationBalance; }
69-
function getUserDistributed() override external view returns (bool) { return userDistributed; }
64+
function getNodeAddress() override external view returns (address) {return nodeAddress;}
65+
66+
function getNodeFee() override external view returns (uint256) {return nodeFee;}
67+
68+
function getNodeDepositBalance() override external view returns (uint256) {return nodeDepositBalance;}
69+
70+
function getNodeRefundBalance() override external view returns (uint256) {return nodeRefundBalance;}
71+
72+
function getNodeDepositAssigned() override external view returns (bool) {return userDepositAssignedTime != 0;}
73+
74+
function getPreLaunchValue() override external view returns (uint256) {return preLaunchValue;}
75+
76+
function getNodeTopUpValue() override external view returns (uint256) {return nodeDepositBalance.sub(preLaunchValue);}
77+
78+
function getVacant() override external view returns (bool) {return vacant;}
79+
80+
function getPreMigrationBalance() override external view returns (uint256) {return preMigrationBalance;}
81+
82+
function getUserDistributed() override external view returns (bool) {return userDistributed;}
7083

7184
// User deposit detail getters
7285
function getUserDepositBalance() override public view returns (uint256) {
@@ -76,9 +89,12 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
7689
return userDepositBalanceLegacy;
7790
}
7891
}
79-
function getUserDepositAssigned() override external view returns (bool) { return userDepositAssignedTime != 0; }
80-
function getUserDepositAssignedTime() override external view returns (uint256) { return userDepositAssignedTime; }
81-
function getTotalScrubVotes() override external view returns (uint256) { return totalScrubVotes; }
92+
93+
function getUserDepositAssigned() override external view returns (bool) {return userDepositAssignedTime != 0;}
94+
95+
function getUserDepositAssignedTime() override external view returns (uint256) {return userDepositAssignedTime;}
96+
97+
function getTotalScrubVotes() override external view returns (uint256) {return totalScrubVotes;}
8298

8399
/// @dev Prevent direct calls to this contract
84100
modifier onlyInitialised() {
@@ -180,7 +196,7 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
180196
require(status >= MinipoolStatus.Initialised && status <= MinipoolStatus.Staking, "The user deposit can only be assigned while initialised, in prelaunch, or staking");
181197
require(userDepositAssignedTime == 0, "The user deposit has already been assigned");
182198
// Progress initialised minipool to prelaunch
183-
if (status == MinipoolStatus.Initialised) { setStatus(MinipoolStatus.Prelaunch); }
199+
if (status == MinipoolStatus.Initialised) {setStatus(MinipoolStatus.Prelaunch);}
184200
// Update user deposit details
185201
userDepositBalance = msg.value;
186202
userDepositAssignedTime = block.timestamp;
@@ -196,6 +212,8 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
196212

197213
/// @notice Refund node ETH refinanced from user deposited ETH
198214
function refund() override external onlyMinipoolOwnerOrWithdrawalAddress(msg.sender) onlyInitialised {
215+
// Prevent vacant minipools from calling
216+
require(vacant == false, "Vacant minipool cannot refund");
199217
// Check refund balance
200218
require(nodeRefundBalance > 0, "No amount of the node deposit is available for refund");
201219
// If this minipool was distributed by a user, force finalisation on the node operator
@@ -417,7 +435,7 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
417435
function beginUserDistribute() override external onlyInitialised {
418436
require(status == MinipoolStatus.Staking, "Minipool must be staking");
419437
uint256 totalBalance = address(this).balance.sub(nodeRefundBalance);
420-
require (totalBalance >= 8 ether, "Balance too low");
438+
require(totalBalance >= 8 ether, "Balance too low");
421439
// Prevent calls resetting distribute time before window has passed
422440
RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool"));
423441
uint256 timeElapsed = block.timestamp.sub(userDistributeTime);
@@ -432,7 +450,7 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
432450
RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool"));
433451
// Calculate if time elapsed since call to `beginUserDistribute` is within the allowed window
434452
uint256 timeElapsed = block.timestamp.sub(userDistributeTime);
435-
return(rocketDAOProtocolSettingsMinipool.isWithinUserDistributeWindow(timeElapsed));
453+
return (rocketDAOProtocolSettingsMinipool.isWithinUserDistributeWindow(timeElapsed));
436454
}
437455

438456
/// @notice Allows the owner of this minipool to finalise it after a user has manually distributed the balance
@@ -611,7 +629,7 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
611629
uint256 quorum = rocketDAONode.getMemberCount().mul(rocketDAONodeTrustedSettingsMinipool.getScrubQuorum()).div(calcBase);
612630
if (totalScrubVotes.add(1) > quorum) {
613631
// Slash RPL equal to minimum stake amount (if enabled)
614-
if (!vacant && rocketDAONodeTrustedSettingsMinipool.getScrubPenaltyEnabled()){
632+
if (!vacant && rocketDAONodeTrustedSettingsMinipool.getScrubPenaltyEnabled()) {
615633
RocketNodeStakingInterface rocketNodeStaking = RocketNodeStakingInterface(getContractAddress("rocketNodeStaking"));
616634
RocketDAOProtocolSettingsNodeInterface rocketDAOProtocolSettingsNode = RocketDAOProtocolSettingsNodeInterface(getContractAddress("rocketDAOProtocolSettingsNode"));
617635
RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool"));
@@ -620,9 +638,9 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
620638
// In prelaunch userDepositBalance hasn't been set so we calculate it as 32 ETH - bond amount
621639
rocketNodeStaking.slashRPL(
622640
nodeAddress,
623-
launchAmount.sub(nodeDepositBalance)
624-
.mul(rocketDAOProtocolSettingsNode.getMinimumPerMinipoolStake())
625-
.div(calcBase)
641+
launchAmount.sub(nodeDepositBalance)
642+
.mul(rocketDAOProtocolSettingsNode.getMinimumPerMinipoolStake())
643+
.div(calcBase)
626644
);
627645
}
628646
// Dissolve this minipool, recycling ETH back to deposit pool
@@ -646,20 +664,25 @@ contract RocketMinipoolDelegate is RocketMinipoolStorageLayout, RocketMinipoolIn
646664
// Approve reduction and handle external state changes
647665
RocketMinipoolBondReducerInterface rocketBondReducer = RocketMinipoolBondReducerInterface(getContractAddress("rocketMinipoolBondReducer"));
648666
uint256 previousBond = nodeDepositBalance;
649-
uint256 newBondAmount = rocketBondReducer.reduceBondAmount();
667+
uint256 newBond = rocketBondReducer.reduceBondAmount();
650668
// Update user/node balances
651-
userDepositBalance = getUserDepositBalance().add(previousBond.sub(newBondAmount));
652-
nodeDepositBalance = newBondAmount;
669+
userDepositBalance = getUserDepositBalance().add(previousBond.sub(newBond));
670+
nodeDepositBalance = newBond;
653671
// Reset node fee to current network rate
654672
RocketNetworkFeesInterface rocketNetworkFees = RocketNetworkFeesInterface(getContractAddress("rocketNetworkFees"));
655-
nodeFee = rocketNetworkFees.getNodeFee();
673+
uint256 prevFee = nodeFee;
674+
uint256 newFee = rocketNetworkFees.getNodeFee();
675+
nodeFee = newFee;
676+
// Update staking minipool counts and fee numerator
677+
RocketMinipoolManagerInterface rocketMinipoolManager = RocketMinipoolManagerInterface(getContractAddress("rocketMinipoolManager"));
678+
rocketMinipoolManager.updateNodeStakingMinipoolCount(previousBond, newBond, prevFee, newFee);
656679
// Break state to prevent rollback exploit
657680
if (depositType != MinipoolDeposit.Variable) {
658-
userDepositBalanceLegacy = 2**256-1;
681+
userDepositBalanceLegacy = 2 ** 256 - 1;
659682
depositType = MinipoolDeposit.Variable;
660683
}
661684
// Emit event
662-
emit BondReduced(previousBond, newBondAmount, block.timestamp);
685+
emit BondReduced(previousBond, newBond, block.timestamp);
663686
}
664687

665688
/// @dev Distributes the current contract balance based on capital ratio and node fee

‎test/minipool/minipool-vacant-tests.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,14 @@ export default function() {
155155
});
156156

157157

158+
it(printTitle('node operator', 'cannot call refund while vacant'), async () => {
159+
// Create a vacant minipool with current balance of 33
160+
let minipool = await createVacantMinipool('8'.ether, {from: node}, null, '33'.ether);
161+
// Try to refund
162+
await shouldRevert(refund(minipool, { from: node }), 'Was able to refund', 'Vacant minipool cannot refund');
163+
});
164+
165+
158166
it(printTitle('node operator', 'can not create a vacant minipool with an existing pubkey'), async () => {
159167
// Create minipool with a pubkey
160168
const pubkey = getValidatorPubkey();

0 commit comments

Comments
 (0)