Lines of code
<https://github.com/multisig-labs/gogopool/blob/4bcef8b1d4e595c9ba41a091b2ebf1b45858f022/contracts/contract/Staking.sol#L125&gt;
<https://github.com/multisig-labs/gogopool/blob/4bcef8b1d4e595c9ba41a091b2ebf1b45858f022/contracts/contract/ClaimNodeOp.sol#L78&gt;

Vulnerability details

Impact

Proof of Concept

The fix for M-19 is to get rid of the miniCount

code-423n4/2022-12-gogopool-findings#235

in calculateAndDistributeRewards function, however, the logic below is added:

// check if their rewards time should be reset
if (staking.getAVAXAssigned(stakerAddr) == 0) {
  staking.setRewardsStartTime(stakerAddr, 0);
}

However, I think when getAVAXAssigned goes to 0 or close to 0, RewardsStartTime should be set to 0

/// @notice The amount of AVAX a given staker is assigned by the protocol (for minipool creation)
/// @param stakerAddr The C-chain address of a GGP staker in the protocol
function getAVAXAssigned(address stakerAddr) public view returns (uint256) {
	int256 stakerIndex = getIndexOf(stakerAddr);
	return getUint(keccak256(abi.encodePacked("staker.item", stakerIndex, ".avaxAssigned")));
}

/// @notice Increase the amount of AVAX a given staker is assigned by the protocol (for minipool creation)
/// @param stakerAddr The C-chain address of a GGP staker in the protocol
function increaseAVAXAssigned(address stakerAddr, uint256 amount) public onlySpecificRegisteredContract("MinipoolManager", msg.sender) {
	int256 stakerIndex = requireValidStaker(stakerAddr);
	addUint(keccak256(abi.encodePacked("staker.item", stakerIndex, ".avaxAssigned")), amount);
}

/// @notice Decrease the amount of AVAX a given staker is assigned by the protocol (for minipool creation)
/// @param stakerAddr The C-chain address of a GGP staker in the protocol
function decreaseAVAXAssigned(address stakerAddr, uint256 amount) public onlySpecificRegisteredContract("MinipoolManager", msg.sender) {
	int256 stakerIndex = requireValidStaker(stakerAddr);
	subUint(keccak256(abi.encodePacked("staker.item", stakerIndex, ".avaxAssigned")), amount);
}

I think when decreaseAVAXAssigned is called and modify the staker’s balance to 0, setRewardsStartTime should be set to 0 instead of waiting for
Multisig to trigger calculateAndDistributeRewards.

decreaseAVAXAssigned is called in recordStakingEnd, cancel or recordStakerEnd

if after decreaseAVAXAssigned is called but the reward start is not reset,

the function below would return incorrect state:

/// @notice Determines if a staker is eligible for the upcoming rewards cycle
/// @dev Eligiblity: time in protocol (secs) > RewardsEligibilityMinSeconds. Rialto will call this.
function isEligible(address stakerAddr) external view returns (bool) {
	Staking staking = Staking(getContractAddress("Staking"));
	uint256 rewardsStartTime = staking.getRewardsStartTime(stakerAddr);
	uint256 elapsedSecs = (block.timestamp - rewardsStartTime);
	ProtocolDAO dao = ProtocolDAO(getContractAddress("ProtocolDAO"));
	return (rewardsStartTime != 0 && elapsedSecs >= dao.getRewardsEligibilityMinSeconds() && staking.getAVAXValidatingHighWater(stakerAddr) > 0);
}

an account with 0 assigned amount of AVAX should not be eligible for reward, the Rialto may incorrectly call isEligible and think that an account is eligible for reward because the rewardsStartTime != 0 and then push reward to the account that has no assigned amount of AVAX balance.

Tools Used

Manual Review

Recommended Mitigation Steps

I think recommend the protocol reeset rewardStartTime in recordStakingEnd, cancel or recordStakerEnd when decreaseAVAXAssigned is called to remove the AVAX balance. I also recommend the protocol revisit the isEligible function call and check if the address has AVAX assigned balance.

	return (
		rewardsStartTime != 0 &&
		elapsedSecs >= dao.getRewardsEligibilityMinSeconds() && 
		staking.getAVAXValidatingHighWater(stakerAddr) > 0 &&
		staking.getAVAXAssigned(stakerAddr) > 0
	);

We are adding the check staking.getAVAXAssigned(stakerAddr) > 0

---

The text was updated successfully, but these errors were encountered:

All reactions