Adds rules for solhint, adapts code for unified style, precommit hook…

… for solhint

.pre-commit-config.yaml

@@ -13,3 +13,16 @@ repos:
     rev: v4.3.21
     hooks:
     -   id: isort
+
+-   repo: local
+    hooks:
+    -   id: lint-sol
+        name: "lint solidity"
+        description: "Checks Solidity code according to the package's linter configuration"
+        language: node
+        entry: solhint
+        files: '\.sol$'
+        args:
+        - --config=./.solhint.json
+        - --ignore-path=./.solhintignore
+        - ./contracts/**/*.sol

.solhint.json

@@ -2,8 +2,44 @@
   "extends": "solhint:recommended",
   "plugins": [],
   "rules": {
+	"explicit-types": ["error","explicit"],
+	"no-console": "error",
+	"no-empty-blocks": "error",
+	"compiler-version": ["error", "^0.8.0"],
+    "max-states-count": ["warn", 20],
+    "no-unused-vars": "error",
+    "reason-string": ["error", {"maxLength": 250}],
+    "constructor-syntax": "error",
+    "quotes": ["error","double"],
+	"const-name-snakecase": "error",
+	"contract-name-camelcase": "error",
+    "event-name-camelcase": "error",
+    "func-name-mixedcase": "error",
+	"immutable-vars-naming": ["error",{"immutablesAsConstants":false}],
+	"modifier-name-mixedcase": "error",
+	"var-name-mixedcase": "error",
+	"named-parameters-mapping": "warn",
+    "use-forbidden-name": "error",
+    "imports-on-top": "error",
+    "ordering": "warn",
+    "visibility-modifier-order": "error",
+    "avoid-call-value": "error",
+    "avoid-low-level-calls": "error",
+    "avoid-sha3": "error",
     "avoid-suicide": "error",
-    "avoid-sha3": "warn",
-    "compiler-version": ["error", "^0.8.0"]
+    "avoid-throw": "error",
+    "avoid-tx-origin": "error",
+    "check-send-result": "error",
+    "func-visibility": ["error", {"ignoreConstructors": true}],
+    "multiple-sends": "error",
+    "no-complex-fallback": "error",
+    "no-inline-assembly": "off",
+    "no-unused-import": "error",
+    "not-rely-on-block-hash": "error",
+    "not-rely-on-time": "off",
+    "reentrancy": "error",
+    "state-visibility": "error",
+	"no-global-import": "off",
+	"func-named-parameters": "off"
   }
 }

.solhintignore

@@ -0,0 +1,5 @@
+**/contracts/contracts/proxy/
+**/contracts/contracts/StakingEscrow.sol
+**/contracts/contracts/NuCypherToken.sol
+**/contracts/test/proxy/
+**/contracts/test/StakingEscrowTestSet.sol

contracts/contracts/Adjudicator.sol

@@ -30,10 +30,6 @@ contract Adjudicator {
         address indexed stakingProvider
     );
 
-    // used only for upgrading
-    bytes32 constant RESERVED_CAPSULE_AND_CFRAG_BYTES = bytes32(0);
-    address constant RESERVED_ADDRESS = address(0);
-
     SignatureVerifier.HashAlgorithm public immutable hashAlgorithm;
     uint256 public immutable basePenalty;
     uint256 public immutable penaltyHistoryCoefficient;

contracts/contracts/IStakingEscrow.sol

@@ -5,11 +5,16 @@ pragma solidity ^0.8.0;
 import "./NuCypherToken.sol";
 
 interface IStakingEscrow {
+    function depositFromWorkLock(address, uint256, uint16) external;
+    function setWorkMeasurement(address, bool) external returns (uint256);
+    function setSnapshots(bool _enableSnapshots) external;
+    function withdraw(uint256 _value) external;
+    function slashStaker(address, uint256, address, uint256) external;
+
     function token() external view returns (NuCypherToken);
     function secondsPerPeriod() external view returns (uint32);
     function stakerFromWorker(address) external view returns (address);
     function getAllTokens(address) external view returns (uint256);
-    function slashStaker(address, uint256, address, uint256) external;
     function genesisSecondsPerPeriod() external view returns (uint32);
     function getPastDowntimeLength(address) external view returns (uint256);
     function findIndexOfPastDowntime(address, uint16) external view returns (uint256);
@@ -19,8 +24,4 @@ interface IStakingEscrow {
     function maxAllowableLockedTokens() external view returns (uint256);
     function minAllowableLockedTokens() external view returns (uint256);
     function getCompletedWork(address) external view returns (uint256);
-    function depositFromWorkLock(address, uint256, uint16) external;
-    function setWorkMeasurement(address, bool) external returns (uint256);
-    function setSnapshots(bool _enableSnapshots) external;
-    function withdraw(uint256 _value) external;
 }

contracts/contracts/TACoApplication.sol

@@ -650,6 +650,7 @@ contract TACoApplication is IApplication, OwnableUpgradeable {
         require(isAuthorized(stakingProvider), "No stake associated with the operator");
         StakingProviderInfo storage info = stakingProviderInfo[stakingProvider];
         require(!info.operatorConfirmed, "Operator address is already confirmed");
+        // solhint-disable-next-line avoid-tx-origin
         require(msg.sender == tx.origin, "Only operator with real address can make a confirmation");
 
         updateRewardInternal(stakingProvider);

contracts/contracts/TestnetThresholdStaking.sol

@@ -69,7 +69,7 @@ contract TestnetThresholdStaking is Ownable {
         info.nuInTStake = _nuInTStake;
     }
 
-    function authorizedStake(address _stakingProvider, address _application) external view returns (uint96) {
+    function authorizedStake(address /* _stakingProvider */, address /* _application */) external view returns (uint96) {
         return 0;
     }
 

contracts/contracts/coordination/Coordinator.sol

@@ -72,16 +72,16 @@ contract Coordinator is AccessControlDefaultAdminRules {
 
     bytes32 public constant INITIATOR_ROLE = keccak256("INITIATOR_ROLE");
 
-    mapping(address => ParticipantKey[]) keysHistory;
+    mapping(address => ParticipantKey[]) internal keysHistory;
 
     IAccessControlApplication public immutable application;
 
     Ritual[] public rituals;
     uint32 public timeout;
     uint16 public maxDkgSize;
     bool public isInitiationPublic;
-    IFeeModel feeModel;  // TODO: Consider making feeModel specific to each ritual
-    IReimbursementPool reimbursementPool;
+    IFeeModel internal feeModel;  // TODO: Consider making feeModel specific to each ritual
+    IReimbursementPool internal reimbursementPool;
     uint256 public totalPendingFees;
     mapping(uint256 => uint256) public pendingFees;
 
@@ -120,6 +120,7 @@ contract Coordinator is AccessControlDefaultAdminRules {
             return RitualState.AWAITING_TRANSCRIPTS;
         } else if (ritual.totalAggregations < ritual.dkgSize) {
             return RitualState.AWAITING_AGGREGATIONS;
+        // solhint-disable-next-line no-empty-blocks
         } else {
             // TODO: Is it possible to reach this state?
             //   - No public key
@@ -143,10 +144,10 @@ contract Coordinator is AccessControlDefaultAdminRules {
         emit ParticipantPublicKeySet(lastRitualId, provider, _publicKey);
     }
 
-    function getProviderPublicKey(address _provider, uint _ritualId) external view returns (BLS12381.G2Point memory) {
+    function getProviderPublicKey(address _provider, uint256 _ritualId) external view returns (BLS12381.G2Point memory) {
         ParticipantKey[] storage participantHistory = keysHistory[_provider];
 
-        for (uint i = participantHistory.length - 1; i >= 0; i--) {
+        for (uint256 i = participantHistory.length - 1; i >= 0; i--) {
             if (participantHistory[i].lastRitualId <= _ritualId) {
                 return participantHistory[i].publicKey;
             }
@@ -347,9 +348,9 @@ contract Coordinator is AccessControlDefaultAdminRules {
         Ritual storage ritual,
         address provider
     ) internal view returns (Participant storage) {
-        uint length = ritual.participant.length;
+        uint256 length = ritual.participant.length;
         // TODO: Improve with binary search
-        for (uint i = 0; i < length; i++) {
+        for (uint256 i = 0; i < length; i++) {
             Participant storage participant = ritual.participant[i];
             if (participant.provider == provider) {
                 return participant;

contracts/contracts/coordination/StakeInfo.sol

@@ -21,7 +21,7 @@ contract StakeInfo is AccessControl, IUpdatableStakeInfo, IAccessControlApplicat
     }
 
     constructor(address[] memory updaters){
-        for(uint i = 0; i < updaters.length; i++){
+        for(uint256 i = 0; i < updaters.length; i++){
             _grantRole(UPDATE_ROLE, updaters[i]);
         }
     }
@@ -71,7 +71,7 @@ contract StakeInfo is AccessControl, IUpdatableStakeInfo, IAccessControlApplicat
 
     function batchUpdate(bytes32[] calldata updateInfo) external override onlyRole(UPDATE_ROLE) {
         require(updateInfo.length % 2 == 0, "bad length");
-        for(uint i = 0; i < updateInfo.length; i += 2){
+        for(uint256 i = 0; i < updateInfo.length; i += 2){
             bytes32 word0 = updateInfo[i];
             bytes32 word1 = updateInfo[i + 1];
 

contracts/contracts/lib/ReEncryptionValidator.sol

@@ -29,15 +29,15 @@ library ReEncryptionValidator {
     //------------------------------//
 
     // Base field order
-    uint256 constant FIELD_ORDER = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F;
+    uint256 internal constant FIELD_ORDER = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFC2F;
 
     // -2 mod FIELD_ORDER
-    uint256 constant MINUS_2 = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2d;
+    uint256 internal constant MINUS_2 = 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2d;
 
     // (-1/2) mod FIELD_ORDER
-    uint256 constant MINUS_ONE_HALF = 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffff7ffffe17;
-
+    uint256 internal constant MINUS_ONE_HALF = 0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffff7ffffe17;
 
+    /* solhint-disable var-name-mixedcase */
     //
 
     /**
@@ -414,9 +414,9 @@ library ReEncryptionValidator {
     /// @param Q An EC point in affine coordinates
     /// @return R An EC point in Jacobian coordinates with the sum, represented by an array of 3 uint256
     function addAffineJacobian(
-    	uint[2] memory P,
-    	uint[2] memory Q
-    ) internal pure returns (uint[3] memory R) {
+    	uint256[2] memory P,
+    	uint256[2] memory Q
+    ) internal pure returns (uint256[3] memory R) {
 
         uint256 p = FIELD_ORDER;
         uint256 a   = P[0];
@@ -440,7 +440,7 @@ library ReEncryptionValidator {
     /// @notice Point doubling in Jacobian coordinates
     /// @param P An EC point in Jacobian coordinates.
     /// @return Q An EC point in Jacobian coordinates
-    function doubleJacobian(uint[3] memory P) internal pure returns (uint[3] memory Q) {
+    function doubleJacobian(uint256[3] memory P) internal pure returns (uint256[3] memory Q) {
         uint256 z = P[2];
         if (z == 0)
             return Q;

contracts/contracts/lib/SignatureVerifier.sol

@@ -12,7 +12,7 @@ library SignatureVerifier {
     enum HashAlgorithm {KECCAK256, SHA256, RIPEMD160}
 
     // Header for Version E as defined by EIP191. First byte ('E') is also the version
-    bytes25 constant EIP191_VERSION_E_HEADER = "Ethereum Signed Message:\n";
+    bytes25 internal constant EIP191_VERSION_E_HEADER = "Ethereum Signed Message:\n";
 
     /**
     * @notice Recover signer address from hash and signature
@@ -24,6 +24,7 @@ library SignatureVerifier {
         pure
         returns (address)
     {
+        // solhint-disable-next-line reason-string
         require(_signature.length == 65);
 
         bytes32 r;
@@ -39,6 +40,7 @@ library SignatureVerifier {
         if (v < 27) {
             v += 27;
         }
+        // solhint-disable-next-line reason-string
         require(v == 27 || v == 28);
         return ecrecover(_hash, v, r, s);
     }
@@ -88,6 +90,7 @@ library SignatureVerifier {
         pure
         returns (bool)
     {
+        // solhint-disable-next-line reason-string
         require(_publicKey.length == 64);
         return toAddress(_publicKey) == recover(hash(_message, _algorithm), _signature);
     }
@@ -153,6 +156,7 @@ library SignatureVerifier {
         view
         returns (bool)
     {
+        // solhint-disable-next-line reason-string
         require(_publicKey.length == 64);
         return toAddress(_publicKey) == recover(hashEIP191(_message, _version), _signature);
     }

contracts/contracts/lib/Snapshot.sol

@@ -33,7 +33,8 @@ library Snapshot {
             if (uint32(_time) == currentTime) {
                 _self[length - 1] = encodeSnapshot(uint32(_time), uint96(_value));
                 return;
-            } else if (uint32(_time) < currentTime){
+            } else if (uint32(_time) < currentTime){ 
+            // solhint-disable-next-line reason-string
                 revert();
             }
         }

contracts/contracts/lib/UmbralDeserializer.sol

@@ -63,21 +63,22 @@ library UmbralDeserializer {
         bytes5  lostBytes;
     }
 
-    uint256 constant BIGNUM_SIZE = 32;
-    uint256 constant POINT_SIZE = 33;
-    uint256 constant SIGNATURE_SIZE = 64;
-    uint256 constant CAPSULE_SIZE = 2 * POINT_SIZE + BIGNUM_SIZE;
-    uint256 constant CORRECTNESS_PROOF_SIZE = 4 * POINT_SIZE + BIGNUM_SIZE + SIGNATURE_SIZE;
-    uint256 constant CAPSULE_FRAG_SIZE = 3 * POINT_SIZE + BIGNUM_SIZE;
-    uint256 constant FULL_CAPSULE_FRAG_SIZE = CAPSULE_FRAG_SIZE + CORRECTNESS_PROOF_SIZE;
-    uint256 constant PRECOMPUTED_DATA_SIZE = (20 * BIGNUM_SIZE) + 32 + 20 + 5;
+    uint256 internal constant BIGNUM_SIZE = 32;
+    uint256 internal constant POINT_SIZE = 33;
+    uint256 internal constant SIGNATURE_SIZE = 64;
+    uint256 internal constant CAPSULE_SIZE = 2 * POINT_SIZE + BIGNUM_SIZE;
+    uint256 internal constant CORRECTNESS_PROOF_SIZE = 4 * POINT_SIZE + BIGNUM_SIZE + SIGNATURE_SIZE;
+    uint256 internal constant CAPSULE_FRAG_SIZE = 3 * POINT_SIZE + BIGNUM_SIZE;
+    uint256 internal constant FULL_CAPSULE_FRAG_SIZE = CAPSULE_FRAG_SIZE + CORRECTNESS_PROOF_SIZE;
+    uint256 internal constant PRECOMPUTED_DATA_SIZE = (20 * BIGNUM_SIZE) + 32 + 20 + 5;
 
     /**
     * @notice Deserialize to capsule (not activated)
     */
     function toCapsule(bytes memory _capsuleBytes)
         internal pure returns (Capsule memory capsule)
     {
+        // solhint-disable-next-line reason-string
         require(_capsuleBytes.length == CAPSULE_SIZE);
         uint256 pointer = getPointer(_capsuleBytes);
         pointer = copyPoint(pointer, capsule.pointE);
@@ -93,6 +94,7 @@ library UmbralDeserializer {
     function toCorrectnessProof(uint256 _pointer, uint256 _proofBytesLength)
         internal pure returns (CorrectnessProof memory proof)
     {
+        // solhint-disable-next-line reason-string
         require(_proofBytesLength >= CORRECTNESS_PROOF_SIZE);
 
         _pointer = copyPoint(_pointer, proof.pointE2);
@@ -128,6 +130,7 @@ library UmbralDeserializer {
         internal pure returns (CapsuleFrag memory cFrag)
     {
         uint256 cFragBytesLength = _cFragBytes.length;
+        // solhint-disable-next-line reason-string
         require(cFragBytesLength >= FULL_CAPSULE_FRAG_SIZE);
 
         uint256 pointer = getPointer(_cFragBytes);
@@ -146,9 +149,10 @@ library UmbralDeserializer {
     function toPreComputedData(bytes memory _preComputedData)
         internal pure returns (PreComputedData memory data)
     {
+        // solhint-disable-next-line reason-string
         require(_preComputedData.length == PRECOMPUTED_DATA_SIZE);
-        uint256 initial_pointer = getPointer(_preComputedData);
-        uint256 pointer = initial_pointer;
+        uint256 initialPointer = getPointer(_preComputedData);
+        uint256 pointer = initialPointer;
 
         data.pointEyCoord = uint256(getBytes32(pointer));
         pointer += BIGNUM_SIZE;
@@ -225,7 +229,8 @@ library UmbralDeserializer {
         data.lostBytes = bytes5(getBytes32(pointer));
         pointer += 5;
 
-        require(pointer == initial_pointer + PRECOMPUTED_DATA_SIZE);
+        // solhint-disable-next-line reason-string
+        require(pointer == initialPointer + PRECOMPUTED_DATA_SIZE);
     }
 
     // TODO extract to external library if needed (#1500)

contracts/matic/SubscriptionManager.sol

@@ -137,11 +137,11 @@ contract SubscriptionManager is Initializable, AccessControlUpgradeable {
         emit FeeRateUpdated(oldFee, newFee);
     }
 
-    function setFeeRate(uint256 _ratePerSecond) onlyRole(SET_RATE_ROLE) external {
+    function setFeeRate(uint256 _ratePerSecond) external onlyRole(SET_RATE_ROLE) {
         _setFeeRate(_ratePerSecond);
     }
 
-    function sweep(address payable recipient) onlyRole(WITHDRAW_ROLE) external {
+    function sweep(address payable recipient) external onlyRole(WITHDRAW_ROLE) {
         uint256 balance = address(this).balance;
         (bool sent, ) = recipient.call{value: balance}("");
         require(sent, "Failed transfer");

contracts/test/AdjudicatorTestSet.sol

@@ -16,7 +16,7 @@ contract TACoApplicationForAdjudicatorMock {
     uint32 public immutable secondsPerPeriod = 1;
     mapping (address => uint96) public stakingProviderInfo;
     mapping (address => uint256) public rewardInfo;
-    mapping (address => address) _stakingProviderFromOperator;
+    mapping (address => address) internal _stakingProviderFromOperator;
 
     function stakingProviderFromOperator(address _operator) public view returns (address) {
         return _stakingProviderFromOperator[_operator];