Address some static-analyzer and fuzzing errors

Makefile

@@ -17,7 +17,7 @@ analyze:
 	# rewind time by up to 24 hours or, 1 hr into the future.
 	slither . \
 		--filter-paths node_modules\|contracts/test \
-		--exclude timestamp
+		--exclude timestamp,naming-convention,assembly
 
 fuzz:
 	echidna . --contract ServiceNodeContributionEchidnaTest --config echidna-local.config.yml

contracts/ServiceNodeContribution.sol

@@ -289,23 +289,29 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
         // NOTE: Check contract collateralisation _after_ the amount is
         // committed to the contract to ensure contribution sums are all
         // accounted for.
-        if ((totalContribution() + totalReservedContribution()) > stakingRequirement)
-            revert ContributionExceedsStakingRequirement(totalContribution(), totalReservedContribution(), stakingRequirement);
+        uint256 currTotalContribution    = totalContribution();
+        uint256 currReservedContribution = totalReservedContribution();
+        if ((currTotalContribution + currReservedContribution) > stakingRequirement)
+            revert ContributionExceedsStakingRequirement(currTotalContribution, currReservedContribution, stakingRequirement);
 
         if (_contributorAddresses.length > maxContributors)
             revert MaxContributorsExceeded(maxContributors);
 
-        emit NewContribution(caller, amount);
+        // NOTE: Allow finalizing the node if the staking requirement is met
+        // State transition before calling out to external code to mitigate
+        // re-entrancy.
+        if (currTotalContribution == stakingRequirement) {
+            emit Filled(_serviceNodeParams.serviceNodePubkey, operator);
+            status = Status.WaitForFinalized;
+        }
 
         // NOTE: Transfer funds from sender to contract
+        emit NewContribution(caller, amount);
         SENT.safeTransferFrom(caller, address(this), amount);
 
-        // NOTE: Allow finalizing the node if the staking requirement is met
-        if (totalContribution() == stakingRequirement) {
-            emit Filled(_serviceNodeParams.serviceNodePubkey, operator);
-            status = Status.WaitForFinalized;
-            if (!manualFinalize) // Auto finalize if allowed
-                _finalize();
+        // NOTE: Auto finalize the node if valid
+        if (status == Status.WaitForFinalized && !manualFinalize) {
+            _finalize();
         }
     }
 
@@ -348,7 +354,7 @@ contract ServiceNodeContribution is Shared, IServiceNodeContribution {
 
         // NOTE: Remove all reserved contributions
         {
-            IServiceNodeRewards.ReservedContributor[] memory zero;
+            IServiceNodeRewards.ReservedContributor[] memory zero = new IServiceNodeRewards.ReservedContributor[](0);
             _updateReservedContributors(zero);
         }
     }

contracts/test/ServiceNodeContributionEchidnaTest.sol

@@ -214,12 +214,17 @@ contract ServiceNodeContributionEchidnaTest {
         try snContribution.withdrawContribution() {
             // Withdraw can succeed if we are not the operator and we had
             // contributed to the contract
-            assert(snOperator != msg.sender);
+            if (snOperator == msg.sender) {
+                assert(snContribution.contributorAddressesLength() == 0);
+            } else {
+                assert(snContribution.contributorAddressesLength() == (numberContributorsBefore - 1));
+                assert(block.timestamp >= snContribution.WITHDRAWAL_DELAY());
+            }
             assert(contribution > 0);
-            assert(snContribution.contributorAddressesLength() == (numberContributorsBefore - 1));
         } catch {
             assert(numberContributorsBefore == 0);
             assert(contribution == 0);
+            assert(block.timestamp < snContribution.WITHDRAWAL_DELAY());
         }
 
         assert(snContribution.status() != IServiceNodeContribution.Status.Finalized);