In the previous implementation, the new avaxNodeOpAmt and avaxLiquidStakerAmt were being set to mp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt.
The problem is avaxNodeOpRewardAmt is always greater than avaxLiquidStakerRewardAmt.
So assuming there is enough liquidity in the Staking pool, the implementation was in favor of the node operator so that the next cycle can use as much as possible.
But it assumes that there is enough liquidity in the Staking pool to be used for the new cycle and when it is broken, some unexpected problems arise.
Mitigation
PR #43
Now the protocol uses mp.avaxNodeOpAmt + mp.avaxLiquidStakerRewardAmt for the new avaxNodeOpAmt instead of mp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt.
This ensures there is always enough AVAX to start the new cycle.
Although some rewards will just accrue in the Vault and not utilized for the new cycle (decrease efficiency), I confirm that the mitigation resolved the original issue.
Suggestion
If the protocol cares about the efficiency for the node operators, we can check the TokenggAVAX.amountAvailableForStaking() and pull as much as possible. Not a bug but a suggestion for improvements.
C4 issue
M-08: Recreated pools receive a wrong AVAX amount due to miscalculated compounded liquid staker amount
Comments
In the previous implementation, the new
avaxNodeOpAmt
andavaxLiquidStakerAmt
were being set tomp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt
. The problem isavaxNodeOpRewardAmt
is always greater thanavaxLiquidStakerRewardAmt
. So assuming there is enough liquidity in the Staking pool, the implementation was in favor of the node operator so that the next cycle can use as much as possible. But it assumes that there is enough liquidity in the Staking pool to be used for the new cycle and when it is broken, some unexpected problems arise.Mitigation
PR #43 Now the protocol uses
mp.avaxNodeOpAmt + mp.avaxLiquidStakerRewardAmt
for the newavaxNodeOpAmt
instead ofmp.avaxNodeOpAmt + mp.avaxNodeOpRewardAmt
. This ensures there is always enough AVAX to start the new cycle. Although some rewards will just accrue in the Vault and not utilized for the new cycle (decrease efficiency), I confirm that the mitigation resolved the original issue.Suggestion
If the protocol cares about the efficiency for the node operators, we can check the
TokenggAVAX.amountAvailableForStaking()
and pull as much as possible. Not a bug but a suggestion for improvements.Conclusion
LGTM