Data desynchronization

The stakingRequirement variable is set to the corresponding value from the ServiceNodeRewards contract in the constructor and cannot be changed in the future because the stakingRequirement is immutable. The _stakingRequirement variable from the ServiceNodeRewards contract corresponds to this value.

The stakingRequirement is used in the contributeFunds function to determine the sufficient amount of contributions collected — after which, funds will be transferred to the ServiceNodeRewards contract during the registration of the service node.

The addBLSPublicKey function from the ServiceNodeRewards contract expects that the exact _stakingRequirement amount of tokens will be transferred. But unlike the ServiceNodeContribution contract, the _stakingRequirement can be changed by the owner of the contract.

constructor(
    address _stakingRewardsContract,
    uint256 _maxContributors,
    BN256G1.G1Point memory _blsPubkey,
    IServiceNodeRewards.ServiceNodeParams memory _serviceNodeParams
) nzAddr(_stakingRewardsContract) nzUint(_maxContributors) {
    stakingRewardsContract = IServiceNodeRewards(_stakingRewardsContract);
    stakingRequirement = stakingRewardsContract.stakingRequirement();
...
}

If _stakingRequirement from the ServiceNodeRewards contract is changed by the owner, the stakingRequirement value will become obsolete. As a result, for example, if _stakingRequirement was increased, the addBLSPublicKey function will be reverted due to the insufficient number of tokens sent. And because the stakingRequirement cannot be updated, this ServiceNodeContribution contract therefore cannot be used in the future.

This issue has been acknowledged by Session team. Their official response is paraphrased below:

Our solution to this is to immediately reject any further contributions if the staking requirement (or contributors) is detected to have changed. The reason for this is that:

1. The staking requirement and contributors is not expected to change very frequently and forcing operators to shutdown the contract via cancelling and reinitiating is an acceptable cost.

2. If we were to allow the requirements to change, this opens up a series of edge cases where contributors that previously had the correct minimum collaterialisation may suddenly become invalidly collateralised or over collateralised in which case all surrounding contracts have to handle this situation correctly. This significantly increases the permutation of possible cases to be handled.

In either case we will be rejecting any open contracts that have not yet been finalised during the time in which a staking or contributor limit has changed and informing the operator to recreate the contract with the new parameters.

Note that changing these limits does not affect any contracts which have been finalised whereby the responsibility of managing their funds is done in the rewards contract where as-of-current is handled independently from the contribution contract and correctly.