totalETHProcessedPerCollateralizedKnot should not be updated if numberOfCollateralisedSlotOwnersForKnot is zero

Summary

If the total number of slot owners of a knot is zero, the value of totalETHProcessedPerCollateralizedKnot should not change.

Explanation

The value of mapping totalETHProcessedPerCollateralizedKnot for a knot is updated when the knot is registered and everytime function _updateCollateralizedSlotOwnersLiabilitySnapshot is called with unprocessedETHForCurrentKnot > 0 && !isNoLongerPartOfSyndicate[_blsPubKey] && currentSlashedAmount < 4 ether. At knot registration, there is a check that the number of owners is >0. But in function _updateCollateralizedSlotOwnersLiabilitySnapshot, if the number of slot owners is zero, the function still increments the value of totalETHProcessedPerCollateralizedKnot by unprocessedETHForCurrentKnot. This means if this was an error in the registry, the owners would not have their rewards accrued, and would lost them as the system would act as if it had distributed them.

// record so unprocessed goes to zero
totalETHProcessedPerCollateralizedKnot[_blsPubKey] = accumulatedETHPerCollateralizedSlotPerKnot;

Impact

Medium impact. If this was ever to happen, it could mean loss of funds for the knot owners. But the fact that the issue relies in the malfunctioning of an external contract makes me think the correct impact is Medium instead of High.

Property violated

totalETHProcessedPerCollateralizedKnot should not change if numberOfCollateralisedSlotOwnersForKnot is zero.

Recommendation

Either revert if the number of owners is zero (I assume here it should never happen and it is safe to do so) or change lines #563 and #564 with:

if(numberOfCollateralisedSlotOwnersForKnot > 0){
    // record so unprocessed goes to zero
    totalETHProcessedPerCollateralizedKnot[_blsPubKey] = accumulatedETHPerCollateralizedSlotPerKnot;
}

neumoxx / 2023-01-blockswap-fv