The original implementation was not checking if the node operator's GGPStake amount is not less than the slash amount.
So technically it was possible that slashGGP() reverts for various reasons.
It is notable that this made an attack path feasible (H-06 MinipoolManager: node operator can avoid being slashed).
Mitigation
PR #41
This PR includes mitigation for various issues (H-03, H-06, M-13).
Just focusing on the issue M-13, it is now checked if the staker has enough GGPStake.
If the staked GGP is not enough, the whole staked GGP is slashed instead of revert.
Analysis
H-06 reported two scenarios where the reverts can happen.
The first case is when the malicious node op provides a long duration intentionally to trigger revert on slash.
This attack path is not feasible now because there is a sanity check on the duration parameter.
The second case is when the GGP price drops down.
For this case, I wonder if we need to slash AVAX from the node op's AVAXStake as much as the difference slashGGPAmt-staking.getGGPStake(owner).
I know it might be too strict for the node operators but just flagging.
C4 issue
M-13: slashing fails when node operator doesn't have enough staked GGP
Comments
The original implementation was not checking if the node operator's
GGPStake
amount is not less than the slash amount. So technically it was possible thatslashGGP()
reverts for various reasons. It is notable that this made an attack path feasible (H-06 MinipoolManager: node operator can avoid being slashed).Mitigation
PR #41 This PR includes mitigation for various issues (H-03, H-06, M-13). Just focusing on the issue M-13, it is now checked if the staker has enough
GGPStake
. If the staked GGP is not enough, the whole staked GGP is slashed instead of revert.Analysis
H-06 reported two scenarios where the reverts can happen.
AVAXStake
as much as the differenceslashGGPAmt-staking.getGGPStake(owner)
. I know it might be too strict for the node operators but just flagging.Conclusion
LGTM