diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4547ab4c..2dffa166 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/.solhint.json b/.solhint.json index b8b919f3..e0e6e341 100644 --- a/.solhint.json +++ b/.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" } } diff --git a/.solhintignore b/.solhintignore new file mode 100644 index 00000000..6986c444 --- /dev/null +++ b/.solhintignore @@ -0,0 +1,5 @@ +**/contracts/contracts/proxy/ +**/contracts/contracts/StakingEscrow.sol +**/contracts/contracts/NuCypherToken.sol +**/contracts/test/proxy/ +**/contracts/test/StakingEscrowTestSet.sol diff --git a/contracts/contracts/Adjudicator.sol b/contracts/contracts/Adjudicator.sol index a2be80c9..5e5c6dbf 100644 --- a/contracts/contracts/Adjudicator.sol +++ b/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; diff --git a/contracts/contracts/IStakingEscrow.sol b/contracts/contracts/IStakingEscrow.sol index 86896fdb..b74e6aad 100644 --- a/contracts/contracts/IStakingEscrow.sol +++ b/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; } diff --git a/contracts/contracts/TACoApplication.sol b/contracts/contracts/TACoApplication.sol index 952fde43..9ed9e68a 100644 --- a/contracts/contracts/TACoApplication.sol +++ b/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); diff --git a/contracts/contracts/TestnetThresholdStaking.sol b/contracts/contracts/TestnetThresholdStaking.sol index 22ac6c5f..ea3d5d41 100644 --- a/contracts/contracts/TestnetThresholdStaking.sol +++ b/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; } diff --git a/contracts/contracts/coordination/Coordinator.sol b/contracts/contracts/coordination/Coordinator.sol index 895eb636..80b506f3 100644 --- a/contracts/contracts/coordination/Coordinator.sol +++ b/contracts/contracts/coordination/Coordinator.sol @@ -72,7 +72,7 @@ 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; @@ -80,8 +80,8 @@ contract Coordinator is AccessControlDefaultAdminRules { 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; diff --git a/contracts/contracts/coordination/StakeInfo.sol b/contracts/contracts/coordination/StakeInfo.sol index 710e5b3c..501ce3c3 100644 --- a/contracts/contracts/coordination/StakeInfo.sol +++ b/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]; diff --git a/contracts/contracts/lib/ReEncryptionValidator.sol b/contracts/contracts/lib/ReEncryptionValidator.sol index 902f06ce..a73cde9d 100644 --- a/contracts/contracts/lib/ReEncryptionValidator.sol +++ b/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; diff --git a/contracts/contracts/lib/SignatureVerifier.sol b/contracts/contracts/lib/SignatureVerifier.sol index 98a4d386..dd1beefe 100644 --- a/contracts/contracts/lib/SignatureVerifier.sol +++ b/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); } diff --git a/contracts/contracts/lib/Snapshot.sol b/contracts/contracts/lib/Snapshot.sol index bd0e17f1..9aa6e201 100644 --- a/contracts/contracts/lib/Snapshot.sol +++ b/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(); } } diff --git a/contracts/contracts/lib/UmbralDeserializer.sol b/contracts/contracts/lib/UmbralDeserializer.sol index 1891d72a..416d4349 100644 --- a/contracts/contracts/lib/UmbralDeserializer.sol +++ b/contracts/contracts/lib/UmbralDeserializer.sol @@ -63,14 +63,14 @@ 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) @@ -78,6 +78,7 @@ library UmbralDeserializer { 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) diff --git a/contracts/matic/SubscriptionManager.sol b/contracts/matic/SubscriptionManager.sol index b36938f7..a846d30f 100644 --- a/contracts/matic/SubscriptionManager.sol +++ b/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"); diff --git a/contracts/test/AdjudicatorTestSet.sol b/contracts/test/AdjudicatorTestSet.sol index 5e825455..2c292bea 100644 --- a/contracts/test/AdjudicatorTestSet.sol +++ b/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]; diff --git a/contracts/test/LibTestSet.sol b/contracts/test/LibTestSet.sol index 488a43f2..f1458ee8 100644 --- a/contracts/test/LibTestSet.sol +++ b/contracts/test/LibTestSet.sol @@ -187,6 +187,7 @@ contract ReEncryptionValidatorMock { using UmbralDeserializer for bytes; + /* solhint-disable func-name-mixedcase, var-name-mixedcase */ function UMBRAL_PARAMETER_U_SIGN() public pure returns (uint8) { return ReEncryptionValidator.UMBRAL_PARAMETER_U_SIGN; } @@ -270,13 +271,13 @@ contract ReEncryptionValidatorMock { } function addAffineJacobian( - uint[2] memory P, - uint[2] memory Q - ) public pure returns (uint[3] memory) { + uint256[2] memory P, + uint256[2] memory Q + ) public pure returns (uint256[3] memory) { return ReEncryptionValidator.addAffineJacobian(P, Q); } - function doubleJacobian(uint[3] memory P) public pure returns (uint[3] memory) { + function doubleJacobian(uint256[3] memory P) public pure returns (uint256[3] memory) { return ReEncryptionValidator.doubleJacobian(P); } } diff --git a/contracts/test/TACoApplicationTestSet.sol b/contracts/test/TACoApplicationTestSet.sol index 3fbc6ac0..cacb52a9 100644 --- a/contracts/test/TACoApplicationTestSet.sol +++ b/contracts/test/TACoApplicationTestSet.sol @@ -81,6 +81,7 @@ contract ThresholdStakingForTACoApplicationMock { } function authorizedStake(address _stakingProvider, address _application) external view returns (uint96) { + // solhint-disable-next-line reason-string require(_stakingProvider == _application || _application == address(application)); return stakingProviderInfo[_stakingProvider].authorized; } diff --git a/contracts/xchain/PolygonChild.sol b/contracts/xchain/PolygonChild.sol index 166db1e0..9088dad7 100644 --- a/contracts/xchain/PolygonChild.sol +++ b/contracts/xchain/PolygonChild.sol @@ -21,7 +21,9 @@ contract PolygonChild is FxBaseChildTunnel, Ownable { bytes memory data ) internal override validateSender(sender) { emit MessageReceived(sender, data); - (bool success, /* returnId */ ) = stakeInfoAddress.call(data); + //(bool success, /* returnId */ ) = + // solhint-disable-next-line avoid-low-level-calls + stakeInfoAddress.call(data); } function sendMessageToRoot(bytes memory message) public { diff --git a/requirements.txt b/requirements.txt index 87385c98..eec4de04 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ -i https://pypi.org/simple aiohttp==3.8.4 ; python_version >= '3.6' aiosignal==1.3.1 ; python_version >= '3.7' -ape-solidity==0.6.5 +ape-solidity==0.6.7 appnope==0.1.3 ; sys_platform == 'darwin' asn1crypto==1.5.1 asttokens==2.2.1 @@ -128,7 +128,7 @@ tox==4.5.1 tqdm==4.65.0 ; python_version >= '3.7' traitlets==5.9.0 ; python_version >= '3.7' trie==2.1.0 ; python_version >= '3.7' and python_version < '4' -typing-extensions==4.6.0 ; python_version < '3.10' +typing-extensions==4.5.0 ; python_version < '3.10' urllib3==2.0.2 ; python_version >= '3.7' varint==1.0.2 virtualenv==20.23.0 ; python_version >= '3.7'