diff --git a/packages/contracts/src/dollar/core/CreditNft.sol b/packages/contracts/src/dollar/core/CreditNft.sol index 729c77e2f..f500a17c0 100644 --- a/packages/contracts/src/dollar/core/CreditNft.sol +++ b/packages/contracts/src/dollar/core/CreditNft.sol @@ -45,11 +45,15 @@ contract CreditNft is ERC1155Ubiquity, ICreditNft { /// @notice Modifier checks that the method is called by a user with the "CreditNft manager" role modifier onlyCreditNftManager() { + _onlyCreditNftManager(); + _; + } + + function _onlyCreditNftManager() internal view { require( accessControl.hasRole(CREDIT_NFT_MANAGER_ROLE, _msgSender()), - "Caller is not a CreditNft manager" + "CreditNft: not CreditNft manager" ); - _; } /// @notice Ensures initialize cannot be called on the implementation contract @@ -120,7 +124,7 @@ contract CreditNft is ERC1155Ubiquity, ICreditNft { * @dev Should be called prior to any state changing functions */ function updateTotalDebt() public { - bool reachedEndOfExpiredKeys = false; + bool reachedEndOfExpiredKeys=false; uint256 currentBlockNumber = _sortedBlockNumbers.popFront(); uint256 outstandingDebt = _totalOutstandingDebt; //if list is empty, currentBlockNumber will be 0 diff --git a/packages/contracts/src/dollar/core/ERC1155Ubiquity.sol b/packages/contracts/src/dollar/core/ERC1155Ubiquity.sol index 6c211deb5..41d9c94e7 100644 --- a/packages/contracts/src/dollar/core/ERC1155Ubiquity.sol +++ b/packages/contracts/src/dollar/core/ERC1155Ubiquity.sol @@ -39,38 +39,54 @@ abstract contract ERC1155Ubiquity is /// @notice Modifier checks that the method is called by a user with the "Governance minter" role modifier onlyMinter() virtual { + _onlyMinter(); + _; + } + + function _onlyMinter() virtual internal view { require( accessControl.hasRole(GOVERNANCE_TOKEN_MINTER_ROLE, _msgSender()), "ERC1155Ubiquity: not minter" ); - _; - } + } /// @notice Modifier checks that the method is called by a user with the "Governance burner" role modifier onlyBurner() virtual { + _onlyBurner(); + _; + } + + function _onlyBurner() virtual internal view { require( accessControl.hasRole(GOVERNANCE_TOKEN_BURNER_ROLE, _msgSender()), "ERC1155Ubiquity: not burner" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Pauser" role modifier onlyPauser() virtual { + _onlyPauser(); + _; + } + + function _onlyPauser() virtual internal view { require( accessControl.hasRole(PAUSER_ROLE, _msgSender()), "ERC1155Ubiquity: not pauser" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Admin" role modifier onlyAdmin() { + _onlyAdmin(); + _; + } + + function _onlyAdmin() internal view { require( accessControl.hasRole(DEFAULT_ADMIN_ROLE, _msgSender()), "ERC20Ubiquity: not admin" ); - _; } /// @notice Ensures __ERC1155Ubiquity_init cannot be called on the implementation contract @@ -140,7 +156,7 @@ abstract contract ERC1155Ubiquity is bytes memory data ) public virtual onlyMinter { _mint(to, id, amount, data); - totalSupply += amount; + totalSupply = totalSupply + amount; holderBalances[to].add(id); } @@ -159,8 +175,12 @@ abstract contract ERC1155Ubiquity is ) public virtual onlyMinter whenNotPaused { _mintBatch(to, ids, amounts, data); uint256 localTotalSupply = totalSupply; - for (uint256 i = 0; i < ids.length; ++i) { + uint256 idsLength = ids.length; + for (uint256 i; i < idsLength;) { localTotalSupply += amounts[i]; + unchecked{ + ++i; + } } totalSupply = localTotalSupply; holderBalances[to].add(ids); @@ -249,7 +269,7 @@ abstract contract ERC1155Ubiquity is uint256 amount ) internal virtual override whenNotPaused { super._burn(account, id, amount); - totalSupply -= amount; + totalSupply = totalSupply - amount; } /** @@ -267,8 +287,11 @@ abstract contract ERC1155Ubiquity is uint256[] memory amounts ) internal virtual override whenNotPaused { super._burnBatch(account, ids, amounts); - for (uint256 i = 0; i < ids.length; ++i) { - totalSupply -= amounts[i]; + for (uint256 i; i < ids.length;) { + totalSupply = totalSupply - amounts[i]; + unchecked { + ++i; + } } } diff --git a/packages/contracts/src/dollar/core/ERC20Ubiquity.sol b/packages/contracts/src/dollar/core/ERC20Ubiquity.sol index e005cc262..db96cea0f 100644 --- a/packages/contracts/src/dollar/core/ERC20Ubiquity.sol +++ b/packages/contracts/src/dollar/core/ERC20Ubiquity.sol @@ -43,20 +43,28 @@ abstract contract ERC20Ubiquity is /// @notice Modifier checks that the method is called by a user with the "pauser" role modifier onlyPauser() { + _onlyPauser(); + _; + } + + function _onlyPauser() internal view { require( accessControl.hasRole(PAUSER_ROLE, msg.sender), "ERC20Ubiquity: not pauser" ); - _; } /// @notice Modifier checks that the method is called by a user with the "admin" role modifier onlyAdmin() { + _onlyAdmin(); + _; + } + + function _onlyAdmin() internal view { require( accessControl.hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "ERC20Ubiquity: not admin" ); - _; } /// @notice Ensures __ERC20Ubiquity_init cannot be called on the implementation contract diff --git a/packages/contracts/src/dollar/core/StakingShare.sol b/packages/contracts/src/dollar/core/StakingShare.sol index 98482a8b4..bdf9f48c8 100644 --- a/packages/contracts/src/dollar/core/StakingShare.sol +++ b/packages/contracts/src/dollar/core/StakingShare.sol @@ -41,29 +41,41 @@ contract StakingShare is ERC1155Ubiquity, ERC1155URIStorageUpgradeable { /// @notice Modifier checks that the method is called by a user with the "Staking share minter" role modifier onlyMinter() override { + _onlyMinter(); + _; + } + + function _onlyMinter() internal override view { require( accessControl.hasRole(STAKING_SHARE_MINTER_ROLE, msg.sender), "Staking Share: not minter" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Staking share burner" role modifier onlyBurner() override { + _onlyBurner(); + _; + } + + function _onlyBurner() internal override view { require( accessControl.hasRole(STAKING_SHARE_BURNER_ROLE, msg.sender), "Staking Share: not burner" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Pauser" role modifier onlyPauser() override { + _onlyPauser(); + _; + } + + function _onlyPauser() internal override view { require( accessControl.hasRole(PAUSER_ROLE, msg.sender), "Staking Share: not pauser" ); - _; } /// @notice Ensures initialize cannot be called on the implementation contract @@ -124,7 +136,7 @@ contract StakingShare is ERC1155Ubiquity, ERC1155URIStorageUpgradeable { ) public virtual onlyMinter whenNotPaused returns (uint256 id) { id = totalSupply + 1; _mint(to, id, 1, bytes("")); - totalSupply += 1; + totalSupply = totalSupply + 1; holderBalances[to].add(id); Stake storage _stake = _stakes[id]; _stake.minter = to; @@ -250,7 +262,7 @@ contract StakingShare is ERC1155Ubiquity, ERC1155URIStorageUpgradeable { super._burn(account, id, 1); Stake storage _stake = _stakes[id]; require(_stake.lpAmount == 0, "LP <> 0"); - totalSupply -= 1; + totalSupply = totalSupply - 1; } /** diff --git a/packages/contracts/src/dollar/core/UbiquityCreditToken.sol b/packages/contracts/src/dollar/core/UbiquityCreditToken.sol index 021438b08..3ab047fe9 100644 --- a/packages/contracts/src/dollar/core/UbiquityCreditToken.sol +++ b/packages/contracts/src/dollar/core/UbiquityCreditToken.sol @@ -27,20 +27,28 @@ contract UbiquityCreditToken is ERC20Ubiquity { /// @notice Modifier checks that the method is called by a user with the "Credit minter" role modifier onlyCreditMinter() { + _onlyCreditMinter(); + _; + } + + function _onlyCreditMinter() internal view { require( accessControl.hasRole(CREDIT_TOKEN_MINTER_ROLE, _msgSender()), "Credit token: not minter" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Credit burner" role modifier onlyCreditBurner() { + _onlyCreditBurner(); + _; + } + + function _onlyCreditBurner() internal view { require( accessControl.hasRole(CREDIT_TOKEN_BURNER_ROLE, _msgSender()), "Credit token: not burner" ); - _; } /** diff --git a/packages/contracts/src/dollar/core/UbiquityDollarToken.sol b/packages/contracts/src/dollar/core/UbiquityDollarToken.sol index 0c157185d..bcd77186c 100644 --- a/packages/contracts/src/dollar/core/UbiquityDollarToken.sol +++ b/packages/contracts/src/dollar/core/UbiquityDollarToken.sol @@ -39,20 +39,28 @@ contract UbiquityDollarToken is ERC20Ubiquity { /// @notice Modifier checks that the method is called by a user with the "Dollar minter" role modifier onlyDollarMinter() { + _onlyDollarMinter(); + _; + } + + function _onlyDollarMinter() internal view { require( accessControl.hasRole(DOLLAR_TOKEN_MINTER_ROLE, _msgSender()), "Dollar token: not minter" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Dollar burner" role modifier onlyDollarBurner() { + _onlyDollarBurner(); + _; + } + + function _onlyDollarBurner() internal view { require( accessControl.hasRole(DOLLAR_TOKEN_BURNER_ROLE, _msgSender()), "Dollar token: not burner" ); - _; } /** diff --git a/packages/contracts/src/dollar/core/UbiquityGovernanceToken.sol b/packages/contracts/src/dollar/core/UbiquityGovernanceToken.sol index 65184eb82..645cdd1eb 100644 --- a/packages/contracts/src/dollar/core/UbiquityGovernanceToken.sol +++ b/packages/contracts/src/dollar/core/UbiquityGovernanceToken.sol @@ -27,20 +27,28 @@ contract UbiquityGovernanceToken is ERC20Ubiquity { /// @notice Modifier checks that the method is called by a user with the "Governance minter" role modifier onlyGovernanceMinter() { + _onlyGovernanceMinter(); + _; + } + + function _onlyGovernanceMinter() internal view { require( accessControl.hasRole(GOVERNANCE_TOKEN_MINTER_ROLE, _msgSender()), "Governance token: not minter" ); - _; } /// @notice Modifier checks that the method is called by a user with the "Governance burner" role modifier onlyGovernanceBurner() { + _onlyGovernanceBurner(); + _; + } + + function _onlyGovernanceBurner() internal view { require( accessControl.hasRole(GOVERNANCE_TOKEN_BURNER_ROLE, _msgSender()), "Governance token: not burner" ); - _; } /** diff --git a/packages/contracts/src/dollar/facets/DiamondLoupeFacet.sol b/packages/contracts/src/dollar/facets/DiamondLoupeFacet.sol index 2045060a8..152472488 100644 --- a/packages/contracts/src/dollar/facets/DiamondLoupeFacet.sol +++ b/packages/contracts/src/dollar/facets/DiamondLoupeFacet.sol @@ -18,14 +18,14 @@ contract DiamondLoupeFacet is IDiamondLoupe, IERC165 { LibDiamond.DiamondStorage storage ds = LibDiamond.diamondStorage(); uint256 numFacets = ds.facetAddresses.length; facets_ = new Facet[](numFacets); - for (uint256 i = 0; i < numFacets; ) { + for (uint256 i; i < numFacets; ) { address facetAddress_ = ds.facetAddresses[i]; facets_[i].facetAddress = facetAddress_; facets_[i].functionSelectors = ds .facetFunctionSelectors[facetAddress_] .functionSelectors; unchecked { - i++; + ++i; } } } diff --git a/packages/contracts/src/dollar/facets/OwnershipFacet.sol b/packages/contracts/src/dollar/facets/OwnershipFacet.sol index f41d47dc9..c07ba8615 100644 --- a/packages/contracts/src/dollar/facets/OwnershipFacet.sol +++ b/packages/contracts/src/dollar/facets/OwnershipFacet.sol @@ -10,7 +10,7 @@ contract OwnershipFacet is IERC173 { function transferOwnership(address _newOwner) external override { require( (_newOwner != address(0)), - "OwnershipFacet: New owner cannot be the zero address" + "OwnershipFacet: Can't address(0)" ); LibDiamond.enforceIsContractOwner(); LibDiamond.setContractOwner(_newOwner); diff --git a/packages/contracts/src/dollar/libraries/Constants.sol b/packages/contracts/src/dollar/libraries/Constants.sol index 006e1e9c4..21a03fb12 100644 --- a/packages/contracts/src/dollar/libraries/Constants.sol +++ b/packages/contracts/src/dollar/libraries/Constants.sol @@ -61,7 +61,7 @@ bytes32 constant CREDIT_NFT_MANAGER_ROLE = keccak256("CREDIT_NFT_MANAGER_ROLE"); bytes32 constant STAKING_MANAGER_ROLE = keccak256("STAKING_MANAGER_ROLE"); /// @dev Role name for inventive manager -bytes32 constant INCENTIVE_MANAGER_ROLE = keccak256("INCENTIVE_MANAGER"); +bytes32 constant INCENTIVE_MANAGER_ROLE = keccak256("INCENTIVE_MANAGER_ROLE"); /// @dev Role name for Governance token manager bytes32 constant GOVERNANCE_TOKEN_MANAGER_ROLE = keccak256( diff --git a/packages/contracts/src/dollar/libraries/LibAppStorage.sol b/packages/contracts/src/dollar/libraries/LibAppStorage.sol index 6bd153c67..e30cf929d 100644 --- a/packages/contracts/src/dollar/libraries/LibAppStorage.sol +++ b/packages/contracts/src/dollar/libraries/LibAppStorage.sol @@ -80,101 +80,149 @@ contract Modifiers { /// @notice Checks that method is called by a contract owner modifier onlyOwner() { - LibDiamond.enforceIsContractOwner(); + _onlyOwner(); _; } + function _onlyOwner() internal view { + LibDiamond.enforceIsContractOwner(); + } + /// @notice Checks that method is called by address with the `CREDIT_NFT_MANAGER_ROLE` role modifier onlyCreditNftManager() { + _onlyCreditNftManager(); + _; + } + + function _onlyCreditNftManager() internal view { require( LibAccessControl.hasRole(CREDIT_NFT_MANAGER_ROLE, msg.sender), "Caller is not a Credit NFT manager" ); - _; } /// @notice Checks that method is called by address with the `DEFAULT_ADMIN_ROLE` role modifier onlyAdmin() { + _onlyAdmin(); + _; + } + + function _onlyAdmin() internal view { require( LibAccessControl.hasRole(DEFAULT_ADMIN_ROLE, msg.sender), "Manager: Caller is not admin" ); - _; } /// @notice Checks that method is called by address with the `GOVERNANCE_TOKEN_MINTER_ROLE` role modifier onlyMinter() { + _onlyMinter(); + _; + } + + function _onlyMinter() internal view { require( LibAccessControl.hasRole(GOVERNANCE_TOKEN_MINTER_ROLE, msg.sender), "Governance token: not minter" ); - _; } /// @notice Checks that method is called by address with the `GOVERNANCE_TOKEN_BURNER_ROLE` role modifier onlyBurner() { + _onlyBurner(); + _; + } + + function _onlyBurner() internal view { require( LibAccessControl.hasRole(GOVERNANCE_TOKEN_BURNER_ROLE, msg.sender), "Governance token: not burner" ); - _; } /// @notice Modifier to make a function callable only when the contract is not paused modifier whenNotPaused() { - require(!LibAccessControl.paused(), "Pausable: paused"); + _whenNotPaused(); _; } + function _whenNotPaused() internal view { + require(!LibAccessControl.paused(), "Pausable: paused"); + } + /// @notice Modifier to make a function callable only when the contract is paused modifier whenPaused() { - require(LibAccessControl.paused(), "Pausable: not paused"); + _whenPaused(); _; } + function _whenPaused() internal view { + require(LibAccessControl.paused(), "Pausable: not paused"); + } + /// @notice Checks that method is called by address with the `STAKING_MANAGER_ROLE` role modifier onlyStakingManager() { + _onlyStakingManager(); + _; + } + + function _onlyStakingManager() internal view { require( LibAccessControl.hasRole(STAKING_MANAGER_ROLE, msg.sender), "not manager" ); - _; } /// @notice Checks that method is called by address with the `PAUSER_ROLE` role modifier onlyPauser() { + _onlyPauser(); + _; + } + + function _onlyPauser() internal view { require( LibAccessControl.hasRole(PAUSER_ROLE, msg.sender), "not pauser" ); - _; } /// @notice Checks that method is called by address with the `GOVERNANCE_TOKEN_MANAGER_ROLE` role modifier onlyTokenManager() { + _onlyTokenManager(); + _; + } + + function _onlyTokenManager() internal view { require( LibAccessControl.hasRole(GOVERNANCE_TOKEN_MANAGER_ROLE, msg.sender), "MasterChef: not Governance Token manager" ); - _; } /// @notice Checks that method is called by address with the `INCENTIVE_MANAGER_ROLE` role modifier onlyIncentiveAdmin() { + _onlyIncentiveAdmin(); + _; + } + + function _onlyIncentiveAdmin() internal view { require( LibAccessControl.hasRole(INCENTIVE_MANAGER_ROLE, msg.sender), "CreditCalc: not admin" ); - _; } /// @notice Checks that method is called by address with the `CURVE_DOLLAR_MANAGER_ROLE` role modifier onlyDollarManager() { + _onlyDollarManager(); + _; + } + + function _onlyDollarManager() internal view { require( LibAccessControl.hasRole(CURVE_DOLLAR_MANAGER_ROLE, msg.sender), "CurveIncentive: Caller is not Ubiquity Dollar" ); - _; } /// @notice Initializes reentrancy guard on contract deployment diff --git a/packages/contracts/src/dollar/libraries/LibChef.sol b/packages/contracts/src/dollar/libraries/LibChef.sol index 0d2a07003..c600e72e3 100644 --- a/packages/contracts/src/dollar/libraries/LibChef.sol +++ b/packages/contracts/src/dollar/libraries/LibChef.sol @@ -123,8 +123,11 @@ library LibChef { "_stakingShareIDs array not same length" ); - for (uint256 i = 0; i < lgt; ++i) { + for (uint256 i; i < lgt;) { deposit(_tos[i], _amounts[i], _stakingShareIDs[i]); + unchecked { + ++i; + } } } diff --git a/packages/contracts/src/dollar/libraries/LibDiamond.sol b/packages/contracts/src/dollar/libraries/LibDiamond.sol index 9a2546e15..4f3493acf 100644 --- a/packages/contracts/src/dollar/libraries/LibDiamond.sol +++ b/packages/contracts/src/dollar/libraries/LibDiamond.sol @@ -120,7 +120,6 @@ library LibDiamond { for ( uint256 facetIndex; facetIndex < _diamondCut.length; - facetIndex++ ) { IDiamondCut.FacetCutAction action = _diamondCut[facetIndex].action; if (action == IDiamondCut.FacetCutAction.Add) { @@ -141,6 +140,9 @@ library LibDiamond { } else { revert("LibDiamondCut: Incorrect FacetCutAction"); } + unchecked { + ++facetIndex; + } } emit DiamondCut(_diamondCut, _init, _calldata); initializeDiamondCut(_init, _calldata); @@ -174,7 +176,6 @@ library LibDiamond { for ( uint256 selectorIndex; selectorIndex < _functionSelectors.length; - selectorIndex++ ) { bytes4 selector = _functionSelectors[selectorIndex]; address oldFacetAddress = ds @@ -186,6 +187,9 @@ library LibDiamond { ); addFunction(ds, selector, selectorPosition, _facetAddress); selectorPosition++; + unchecked { + ++selectorIndex; + } } } @@ -217,7 +221,6 @@ library LibDiamond { for ( uint256 selectorIndex; selectorIndex < _functionSelectors.length; - selectorIndex++ ) { bytes4 selector = _functionSelectors[selectorIndex]; address oldFacetAddress = ds @@ -230,6 +233,9 @@ library LibDiamond { removeFunction(ds, oldFacetAddress, selector); addFunction(ds, selector, selectorPosition, _facetAddress); selectorPosition++; + unchecked { + ++selectorIndex; + } } } @@ -255,13 +261,15 @@ library LibDiamond { for ( uint256 selectorIndex; selectorIndex < _functionSelectors.length; - selectorIndex++ ) { bytes4 selector = _functionSelectors[selectorIndex]; address oldFacetAddress = ds .selectorToFacetAndPosition[selector] .facetAddress; removeFunction(ds, oldFacetAddress, selector); + unchecked { + ++selectorIndex; + } } } diff --git a/packages/contracts/src/dollar/libraries/LibDirectGovernanceFarmer.sol b/packages/contracts/src/dollar/libraries/LibDirectGovernanceFarmer.sol index 102c86aca..7fed3f2fa 100644 --- a/packages/contracts/src/dollar/libraries/LibDirectGovernanceFarmer.sol +++ b/packages/contracts/src/dollar/libraries/LibDirectGovernanceFarmer.sol @@ -454,10 +454,14 @@ library LibDirectGovernanceFarmer { uint256[] memory idList, uint256 id ) internal pure returns (bool) { - for (uint256 i = 0; i < idList.length; i++) { + uint256 length = idList.length; + for (uint256 i; i < length;) { if (idList[i] == id) { return true; } + unchecked { + ++i; + } } return false; } diff --git a/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol b/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol index 2f2a90a35..c1ac8e206 100644 --- a/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol +++ b/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol @@ -167,6 +167,11 @@ library LibUbiquityPool { * @param collateralIndex Collateral token index */ modifier collateralEnabled(uint256 collateralIndex) { + _collateralEnabled(collateralIndex); + _; + } + + function _collateralEnabled(uint256 collateralIndex) internal view { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); require( poolStorage.isCollateralEnabled[ @@ -174,19 +179,22 @@ library LibUbiquityPool { ], "Collateral disabled" ); - _; } /** * @notice Checks whether a caller is the AMO minter address */ modifier onlyAmoMinter() { + _onlyAmoMinter(); + _; + } + + function _onlyAmoMinter() internal view { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); require( poolStorage.isAmoMinterEnabled[msg.sender], "Not an AMO Minter" ); - _; } //===================== @@ -252,11 +260,14 @@ library LibUbiquityPool { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); uint256 collateralTokensCount = poolStorage.collateralAddresses.length; balanceTally = 0; - for (uint256 i = 0; i < collateralTokensCount; i++) { + for (uint256 i; i < collateralTokensCount;) { balanceTally += freeCollateralBalance(i) .mul(10 ** poolStorage.missingDecimals[i]) .mul(poolStorage.collateralPrices[i]) .div(UBIQUITY_POOL_PRICE_PRECISION); + unchecked { + ++i; + } } } @@ -336,7 +347,7 @@ library LibUbiquityPool { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); require( - poolStorage.isMintPaused[collateralIndex] == false, + !poolStorage.isMintPaused[collateralIndex], "Minting is paused" ); @@ -408,7 +419,7 @@ library LibUbiquityPool { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); require( - poolStorage.isRedeemPaused[collateralIndex] == false, + !poolStorage.isRedeemPaused[collateralIndex], "Redeeming is paused" ); @@ -479,7 +490,7 @@ library LibUbiquityPool { UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage(); require( - poolStorage.isRedeemPaused[collateralIndex] == false, + !poolStorage.isRedeemPaused[collateralIndex], "Redeeming is paused" ); require( @@ -491,7 +502,7 @@ library LibUbiquityPool { "Too soon to collect redemption" ); - bool sendCollateral = false; + bool sendCollateral; if ( poolStorage.redeemCollateralBalances[msg.sender][collateralIndex] > @@ -542,7 +553,7 @@ library LibUbiquityPool { uint256 priceFeedDecimals = priceFeed.decimals(); // validation - require(answer > 0, "Invalid price"); + require(answer != 0, "Invalid price"); require( block.timestamp - updatedAt < poolStorage.collateralPriceFeedStalenessThresholds[ @@ -580,7 +591,7 @@ library LibUbiquityPool { // checks to see if borrowing is paused require( - poolStorage.isBorrowPaused[minterCollateralIndex] == false, + !poolStorage.isBorrowPaused[minterCollateralIndex], "Borrowing is paused" ); diff --git a/packages/contracts/src/dollar/libraries/UintUtils.sol b/packages/contracts/src/dollar/libraries/UintUtils.sol index 63702794a..2bad00005 100644 --- a/packages/contracts/src/dollar/libraries/UintUtils.sol +++ b/packages/contracts/src/dollar/libraries/UintUtils.sol @@ -77,7 +77,7 @@ library UintUtils { return "0x00"; } - uint256 length = 0; + uint256 length; for (uint256 temp = value; temp != 0; temp >>= 8) { unchecked { diff --git a/packages/contracts/src/dollar/utils/SafeAddArray.sol b/packages/contracts/src/dollar/utils/SafeAddArray.sol index 5f5b14246..9e330803e 100644 --- a/packages/contracts/src/dollar/utils/SafeAddArray.sol +++ b/packages/contracts/src/dollar/utils/SafeAddArray.sol @@ -13,10 +13,14 @@ library SafeAddArray { */ function add(uint256[] storage array, uint256 value) internal { // slither-disable-next-line uninitialized-local - for (uint256 i; i < array.length; i++) { + uint256 length = array.length; + for (uint256 i=0; i < length;) { if (array[i] == value) { return; } + unchecked { + ++i; + } } array.push(value); } @@ -28,9 +32,9 @@ library SafeAddArray { */ function add(uint256[] storage array, uint256[] memory values) internal { // slither-disable-next-line uninitialized-local - for (uint256 i; i < values.length; ) { - bool exist = false; - for (uint256 j = 0; j < array.length; j++) { + for (uint256 i=0; i < values.length; ) { + bool exist=false; + for (uint256 j; j < array.length; j++) { if (array[j] == values[i]) { exist = true; break; diff --git a/packages/contracts/src/ubiquistick/SimpleBond.sol b/packages/contracts/src/ubiquistick/SimpleBond.sol index 759ec85e8..c77ede055 100644 --- a/packages/contracts/src/ubiquistick/SimpleBond.sol +++ b/packages/contracts/src/ubiquistick/SimpleBond.sol @@ -55,11 +55,19 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { /// @notice onlySticker : no NFT stick address defined OR sender has at least one NFT Stick modifier onlySticker() { + _onlySticker(); + _; + } + + function _onlySticker() internal view { require( sticker == address(0) || IERC721(sticker).balanceOf(msg.sender) > 0, "Not NFT Stick owner" ); - _; + } + + function _NotZeroAddress(address _address) internal pure{ + require(_address != address(0), "Invalid address"); } /// @notice Set sticker @@ -76,7 +84,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { uint256 vestingBlocks_, address treasury_ ) { - require(tokenRewards_ != address(0), "Invalid Reward token"); + _NotZeroAddress(tokenRewards_); tokenRewards = tokenRewards_; setVestingBlocks(vestingBlocks_); setTreasury(treasury_); @@ -89,7 +97,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { address token, uint256 tokenRewardsRatio ) public override onlyOwner { - require(token != address(0), "Invalid Reward token"); + _NotZeroAddress(token); rewardsRatio[token] = tokenRewardsRatio; emit LogSetRewards(token, tokenRewardsRatio); @@ -100,14 +108,14 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { function setVestingBlocks( uint256 vestingBlocks_ ) public override onlyOwner { - require(vestingBlocks_ > 0, "Invalid Vesting blocks number"); + require(vestingBlocks_ != 0, "Invalid Vesting blocks number"); vestingBlocks = vestingBlocks_; } /// @notice Set treasury address /// @param treasury_ treasury address function setTreasury(address treasury_) public override onlyOwner { - require(treasury_ != address(0), "Invalid Treasury address"); + _NotZeroAddress(treasury_); treasury = treasury_; } @@ -129,7 +137,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { address token, uint256 amount ) public override whenNotPaused onlySticker returns (uint256 bondId) { - require(rewardsRatio[token] > 0, "Token not allowed"); + require(rewardsRatio[token] != 0, "Token not allowed"); // @dev throws if not enough allowance or tokens for address // @dev must set token allowance for this smart contract previously @@ -145,7 +153,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { uint256 rewards = (amount * rewardsRatio[token]) / 1_000_000_000; bondState.rewards = rewards; - totalRewards += rewards; + totalRewards = totalRewards + rewards; bondId = bonds[msg.sender].length; bonds[msg.sender].push(bondState); @@ -164,7 +172,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { /// @return claimed Rewards claimed successfully function claim() public override whenNotPaused returns (uint256 claimed) { for ( - uint256 index = 0; + uint256 index; (index < bonds[msg.sender].length); index += 1 ) { @@ -182,7 +190,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { if (claimAmount > 0) { bondState.claimed += claimAmount; - totalClaimedRewards += claimAmount; + totalClaimedRewards = totalClaimedRewards + claimAmount; assert(bondState.claimed <= bondState.rewards); IUAR(tokenRewards).raiseCapital(claimAmount); @@ -220,7 +228,7 @@ contract SimpleBond is ISimpleBond, Ownable, Pausable { uint256 rewardsClaimable ) { - for (uint256 index = 0; index < bonds[addr].length; index += 1) { + for (uint256 index; index < bonds[addr].length; index += 1) { ( uint256 bondRewards, uint256 bondClaimedRewards, diff --git a/packages/contracts/src/ubiquistick/UbiquiStick.sol b/packages/contracts/src/ubiquistick/UbiquiStick.sol index 9f4e7b8ad..5fc5e900e 100644 --- a/packages/contracts/src/ubiquistick/UbiquiStick.sol +++ b/packages/contracts/src/ubiquistick/UbiquiStick.sol @@ -45,10 +45,14 @@ contract UbiquiStick is uint256 private constant _INVISIBLE_TYPE = 2; modifier onlyMinter() { - require(msg.sender == minter, "Not minter"); + _onlyMinter(); _; } + function _onlyMinter() internal view { + require(msg.sender == minter, "Not minter"); + } + constructor() ERC721("The UbiquiStick", "KEY") { setMinter(msg.sender); } @@ -99,8 +103,11 @@ contract UbiquiStick is } function batchSafeMint(address to, uint256 count) public onlyMinter { - for (uint256 i = 0; i < count; i++) { + for (uint256 i; i < count;) { safeMint(to); + unchecked { + ++i; + } } } diff --git a/packages/contracts/src/ubiquistick/UbiquiStickSale.sol b/packages/contracts/src/ubiquistick/UbiquiStickSale.sol index 9a2069223..adccd0a6f 100644 --- a/packages/contracts/src/ubiquistick/UbiquiStickSale.sol +++ b/packages/contracts/src/ubiquistick/UbiquiStickSale.sol @@ -38,13 +38,17 @@ contract UbiquiStickSale is Ownable, ReentrancyGuard { constructor() {} + function _NotZeroAddress(address _address) internal pure{ + require(_address != address(0), "Invalid address"); + } + function setTokenContract(address _newTokenContract) external onlyOwner { - require(_newTokenContract != address(0), "Invalid Address"); + _NotZeroAddress(_newTokenContract); tokenContract = IUbiquiStick(_newTokenContract); } function setFundsAddress(address _address) external onlyOwner { - require(_address != address(0), "Invalid Address"); + _NotZeroAddress(_address); fundsAddress = _address; } @@ -54,7 +58,7 @@ contract UbiquiStickSale is Ownable, ReentrancyGuard { uint256 _count, uint256 _price ) public onlyOwner { - require(_address != address(0), "Invalid Address"); + _NotZeroAddress(_address); _allowances[_address] = Purchase(_count, _price); } @@ -66,8 +70,11 @@ contract UbiquiStickSale is Ownable, ReentrancyGuard { ) external onlyOwner { uint256 count = _addresses.length; - for (uint16 i = 0; i < count; i++) { + for (uint16 i; i < count;) { setAllowance(_addresses[i], _counts[i], _prices[i]); + unchecked { + ++i; + } } } diff --git a/packages/contracts/test/diamond/facets/OwnershipFacet.t.sol b/packages/contracts/test/diamond/facets/OwnershipFacet.t.sol index abb518fa7..579dfbf40 100644 --- a/packages/contracts/test/diamond/facets/OwnershipFacet.t.sol +++ b/packages/contracts/test/diamond/facets/OwnershipFacet.t.sol @@ -38,7 +38,7 @@ contract OwnershipFacetTest is DiamondTestSetup { vm.prank(owner); vm.expectRevert( abi.encodePacked( - "OwnershipFacet: New owner cannot be the zero address" + "OwnershipFacet: Can't address(0)" ) ); ownershipFacet.transferOwnership(address(0)); diff --git a/packages/contracts/test/dollar/core/CreditNft.t.sol b/packages/contracts/test/dollar/core/CreditNft.t.sol index c8cc5f9df..acb1f5ea6 100644 --- a/packages/contracts/test/dollar/core/CreditNft.t.sol +++ b/packages/contracts/test/dollar/core/CreditNft.t.sol @@ -45,7 +45,7 @@ contract CreditNftTest is LocalTestHelper { } function testMintCreditNft_ShouldRevert_WhenNotCreditNftManager() public { - vm.expectRevert("Caller is not a CreditNft manager"); + vm.expectRevert("CreditNft: not CreditNft manager"); creditNft.mintCreditNft(address(0x123), 1, 100); } @@ -67,7 +67,7 @@ contract CreditNftTest is LocalTestHelper { } function testBurnCreditNft_ShouldRevert_WhenNotCreditNftManager() public { - vm.expectRevert("Caller is not a CreditNft manager"); + vm.expectRevert("CreditNft: not CreditNft manager"); creditNft.burnCreditNft(address(0x123), 1, 100); } diff --git a/packages/contracts/test/ubiquistick/UbiquiStickSale.t.sol b/packages/contracts/test/ubiquistick/UbiquiStickSale.t.sol index dabfd25eb..c539623e0 100644 --- a/packages/contracts/test/ubiquistick/UbiquiStickSale.t.sol +++ b/packages/contracts/test/ubiquistick/UbiquiStickSale.t.sol @@ -34,7 +34,7 @@ contract UbiquiStickSaleTest is Test { function testSetTokenContract_ShouldRevert_IfAddressIsZero() public { vm.prank(owner); - vm.expectRevert("Invalid Address"); + vm.expectRevert("Invalid address"); ubiquiStickSale.setTokenContract(address(0)); } @@ -55,7 +55,7 @@ contract UbiquiStickSaleTest is Test { function testSetFundsAddress_ShouldRevert_IfAddressIsZero() public { vm.prank(owner); - vm.expectRevert("Invalid Address"); + vm.expectRevert("Invalid address"); ubiquiStickSale.setFundsAddress(address(0)); } @@ -73,7 +73,7 @@ contract UbiquiStickSaleTest is Test { function testSetAllowance_ShouldRevert_IfAddressIsZero() public { vm.prank(owner); - vm.expectRevert("Invalid Address"); + vm.expectRevert("Invalid address"); ubiquiStickSale.setAllowance(address(0), 1, 1); }