From 6c85298d9bd303dc7cd703d69c7c314c4a22d031 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wei=C3=9Fer=20Hase?= Date: Mon, 3 Jun 2024 14:00:36 +0200 Subject: [PATCH] fix: addressing linter warnings (#49) * fix: addressing linter warnings * fix: rm extra line * fix: reenable linter pre-commit --- .husky/pre-commit | 7 +- .solhint.json | 5 +- .solhint.tests.json | 8 +- .solhintignore | 1 + src/contracts/BColor.sol | 2 +- src/contracts/BConst.sol | 2 +- src/contracts/BFactory.sol | 31 +++---- src/contracts/BMath.sol | 4 +- src/contracts/BNum.sol | 4 +- src/contracts/BPool.sol | 174 +++++++++++++++++++------------------ src/contracts/BToken.sol | 66 ++++++-------- test/unit/BPool.t.sol | 3 +- 12 files changed, 152 insertions(+), 155 deletions(-) create mode 100644 .solhintignore diff --git a/.husky/pre-commit b/.husky/pre-commit index 0dc1b6a2..f567e92a 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -2,7 +2,6 @@ . "$(dirname "$0")/_/husky.sh" # 1. Build the contracts -# 2. Stage build output -# 2. Lint and stage style improvements -# TODO: remember to re-enable linter -yarn build # && npx lint-staged \ No newline at end of file +# 2. Stage build output +# 2. Lint and stage style improvements +yarn build && npx lint-staged \ No newline at end of file diff --git a/.solhint.json b/.solhint.json index a8b002b7..48de62d1 100644 --- a/.solhint.json +++ b/.solhint.json @@ -15,7 +15,8 @@ "no-console": "off", "max-line-length": ["warn", 120], "TODO": "REMOVE_TEMPORARY_LINTER_SETTINGS_BELOW", - "custom-errors": "warn", - "definition-name-capwords": "warn" + "custom-errors": "off", + "one-contract-per-file": "off", + "definition-name-capwords": "off" } } diff --git a/.solhint.tests.json b/.solhint.tests.json index e208b5f4..887fba05 100644 --- a/.solhint.tests.json +++ b/.solhint.tests.json @@ -15,11 +15,13 @@ "named-parameters-function": "off", "no-global-import": "off", "max-states-count": "off", - "private-vars-leading-underscore": ["warn", { "strict": false }], - "ordering": "warn", + "no-unused-vars": "off", + "private-vars-leading-underscore": ["off"], + "ordering": "off", + "state-visibility": "off", "immutable-name-snakecase": "warn", "avoid-low-level-calls": "off", "one-contract-per-file": "off", - "max-line-length": ["warn", 120] + "max-line-length": ["warn", 125] } } diff --git a/.solhintignore b/.solhintignore new file mode 100644 index 00000000..9fdb1048 --- /dev/null +++ b/.solhintignore @@ -0,0 +1 @@ +test/smock/* \ No newline at end of file diff --git a/src/contracts/BColor.sol b/src/contracts/BColor.sol index a29c076d..57d4c0e1 100644 --- a/src/contracts/BColor.sol +++ b/src/contracts/BColor.sol @@ -6,7 +6,7 @@ abstract contract BColor { } contract BBronze is BColor { - function getColor() external view override returns (bytes32) { + function getColor() external pure override returns (bytes32) { return bytes32('BRONZE'); } } diff --git a/src/contracts/BConst.sol b/src/contracts/BConst.sol index ac229218..ade59b97 100644 --- a/src/contracts/BConst.sol +++ b/src/contracts/BConst.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.23; -import './BColor.sol'; +import {BBronze} from './BColor.sol'; contract BConst is BBronze { uint256 public constant BONE = 10 ** 18; diff --git a/src/contracts/BFactory.sol b/src/contracts/BFactory.sol index e2595b08..a601b3ed 100644 --- a/src/contracts/BFactory.sol +++ b/src/contracts/BFactory.sol @@ -3,17 +3,20 @@ pragma solidity 0.8.23; // Builds new BPools, logging their addresses and providing `isBPool(address) -> (bool)` -import './BPool.sol'; +import {BBronze} from './BColor.sol'; +import {BPool} from './BPool.sol'; +import {IERC20} from './BToken.sol'; contract BFactory is BBronze { + mapping(address => bool) internal _isBPool; + address internal _blabs; + event LOG_NEW_POOL(address indexed caller, address indexed pool); event LOG_BLABS(address indexed caller, address indexed blabs); - mapping(address => bool) internal _isBPool; - - function isBPool(address b) external view returns (bool) { - return _isBPool[b]; + constructor() { + _blabs = msg.sender; } function newBPool() external returns (BPool) { @@ -24,16 +27,6 @@ contract BFactory is BBronze { return bpool; } - address internal _blabs; - - constructor() { - _blabs = msg.sender; - } - - function getBLabs() external view returns (address) { - return _blabs; - } - function setBLabs(address b) external { require(msg.sender == _blabs, 'ERR_NOT_BLABS'); emit LOG_BLABS(msg.sender, b); @@ -46,4 +39,12 @@ contract BFactory is BBronze { bool xfer = pool.transfer(_blabs, collected); require(xfer, 'ERR_ERC20_FAILED'); } + + function isBPool(address b) external view returns (bool) { + return _isBPool[b]; + } + + function getBLabs() external view returns (address) { + return _blabs; + } } diff --git a/src/contracts/BMath.sol b/src/contracts/BMath.sol index 39b72e0f..b8ec5b63 100644 --- a/src/contracts/BMath.sol +++ b/src/contracts/BMath.sol @@ -1,7 +1,9 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.23; -import './BNum.sol'; +import {BBronze} from './BColor.sol'; +import {BConst} from './BConst.sol'; +import {BNum} from './BNum.sol'; contract BMath is BBronze, BConst, BNum { /** diff --git a/src/contracts/BNum.sol b/src/contracts/BNum.sol index 9ec52622..b2cfe6fd 100644 --- a/src/contracts/BNum.sol +++ b/src/contracts/BNum.sol @@ -1,8 +1,10 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.23; -import './BConst.sol'; +import {BConst} from './BConst.sol'; +// solhint-disable private-vars-leading-underscore +// solhint-disable named-return-values contract BNum is BConst { function btoi(uint256 a) internal pure returns (uint256) { unchecked { diff --git a/src/contracts/BPool.sol b/src/contracts/BPool.sol index 3827566f..deddf2a2 100644 --- a/src/contracts/BPool.sol +++ b/src/contracts/BPool.sol @@ -1,8 +1,9 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.23; -import './BMath.sol'; -import './BToken.sol'; +import {BBronze} from './BColor.sol'; +import {BMath} from './BMath.sol'; +import {BToken, IERC20} from './BToken.sol'; contract BPool is BBronze, BToken, BMath { struct Record { @@ -12,37 +13,6 @@ contract BPool is BBronze, BToken, BMath { uint256 balance; } - event LOG_SWAP( - address indexed caller, - address indexed tokenIn, - address indexed tokenOut, - uint256 tokenAmountIn, - uint256 tokenAmountOut - ); - - event LOG_JOIN(address indexed caller, address indexed tokenIn, uint256 tokenAmountIn); - - event LOG_EXIT(address indexed caller, address indexed tokenOut, uint256 tokenAmountOut); - - event LOG_CALL(bytes4 indexed sig, address indexed caller, bytes data) anonymous; - - modifier _logs_() { - emit LOG_CALL(msg.sig, msg.sender, msg.data); - _; - } - - modifier _lock_() { - require(!_mutex, 'ERR_REENTRY'); - _mutex = true; - _; - _mutex = false; - } - - modifier _viewlock_() { - require(!_mutex, 'ERR_REENTRY'); - _; - } - bool internal _mutex; address internal _factory; // BFactory address to push token exitFee to @@ -58,6 +28,20 @@ contract BPool is BBronze, BToken, BMath { mapping(address => Record) internal _records; uint256 internal _totalWeight; + event LOG_SWAP( + address indexed caller, + address indexed tokenIn, + address indexed tokenOut, + uint256 tokenAmountIn, + uint256 tokenAmountOut + ); + + event LOG_JOIN(address indexed caller, address indexed tokenIn, uint256 tokenAmountIn); + + event LOG_EXIT(address indexed caller, address indexed tokenOut, uint256 tokenAmountOut); + + event LOG_CALL(bytes4 indexed sig, address indexed caller, bytes data) anonymous; + constructor() { _controller = msg.sender; _factory = msg.sender; @@ -66,59 +50,6 @@ contract BPool is BBronze, BToken, BMath { _finalized = false; } - function isPublicSwap() external view returns (bool) { - return _publicSwap; - } - - function isFinalized() external view returns (bool) { - return _finalized; - } - - function isBound(address t) external view returns (bool) { - return _records[t].bound; - } - - function getNumTokens() external view returns (uint256) { - return _tokens.length; - } - - function getCurrentTokens() external view _viewlock_ returns (address[] memory tokens) { - return _tokens; - } - - function getFinalTokens() external view _viewlock_ returns (address[] memory tokens) { - require(_finalized, 'ERR_NOT_FINALIZED'); - return _tokens; - } - - function getDenormalizedWeight(address token) external view _viewlock_ returns (uint256) { - require(_records[token].bound, 'ERR_NOT_BOUND'); - return _records[token].denorm; - } - - function getTotalDenormalizedWeight() external view _viewlock_ returns (uint256) { - return _totalWeight; - } - - function getNormalizedWeight(address token) external view _viewlock_ returns (uint256) { - require(_records[token].bound, 'ERR_NOT_BOUND'); - uint256 denorm = _records[token].denorm; - return bdiv(denorm, _totalWeight); - } - - function getBalance(address token) external view _viewlock_ returns (uint256) { - require(_records[token].bound, 'ERR_NOT_BOUND'); - return _records[token].balance; - } - - function getSwapFee() external view _viewlock_ returns (uint256) { - return _swapFee; - } - - function getController() external view _viewlock_ returns (address) { - return _controller; - } - function setSwapFee(uint256 swapFee) external _logs_ _lock_ { require(!_finalized, 'ERR_IS_FINALIZED'); require(msg.sender == _controller, 'ERR_NOT_CONTROLLER'); @@ -202,6 +133,7 @@ contract BPool is BBronze, BToken, BMath { } } + // solhint-disable-next-line ordering function unbind(address token) external _logs_ _lock_ { require(msg.sender == _controller, 'ERR_NOT_CONTROLLER'); require(_records[token].bound, 'ERR_NOT_BOUND'); @@ -492,6 +424,59 @@ contract BPool is BBronze, BToken, BMath { return poolAmountIn; } + function isPublicSwap() external view returns (bool) { + return _publicSwap; + } + + function isFinalized() external view returns (bool) { + return _finalized; + } + + function isBound(address t) external view returns (bool) { + return _records[t].bound; + } + + function getNumTokens() external view returns (uint256) { + return _tokens.length; + } + + function getCurrentTokens() external view _viewlock_ returns (address[] memory tokens) { + return _tokens; + } + + function getFinalTokens() external view _viewlock_ returns (address[] memory tokens) { + require(_finalized, 'ERR_NOT_FINALIZED'); + return _tokens; + } + + function getDenormalizedWeight(address token) external view _viewlock_ returns (uint256) { + require(_records[token].bound, 'ERR_NOT_BOUND'); + return _records[token].denorm; + } + + function getTotalDenormalizedWeight() external view _viewlock_ returns (uint256) { + return _totalWeight; + } + + function getNormalizedWeight(address token) external view _viewlock_ returns (uint256) { + require(_records[token].bound, 'ERR_NOT_BOUND'); + uint256 denorm = _records[token].denorm; + return bdiv(denorm, _totalWeight); + } + + function getBalance(address token) external view _viewlock_ returns (uint256) { + require(_records[token].bound, 'ERR_NOT_BOUND'); + return _records[token].balance; + } + + function getSwapFee() external view _viewlock_ returns (uint256) { + return _swapFee; + } + + function getController() external view _viewlock_ returns (address) { + return _controller; + } + // == // 'Underlying' token-manipulation functions make external calls but are NOT locked // You must `_lock_` or otherwise ensure reentry-safety @@ -521,4 +506,21 @@ contract BPool is BBronze, BToken, BMath { function _burnPoolShare(uint256 amount) internal { _burn(amount); } + + modifier _logs_() { + emit LOG_CALL(msg.sig, msg.sender, msg.data); + _; + } + + modifier _lock_() { + require(!_mutex, 'ERR_REENTRY'); + _mutex = true; + _; + _mutex = false; + } + + modifier _viewlock_() { + require(!_mutex, 'ERR_REENTRY'); + _; + } } diff --git a/src/contracts/BToken.sol b/src/contracts/BToken.sol index e83a31a5..31ad2402 100644 --- a/src/contracts/BToken.sol +++ b/src/contracts/BToken.sol @@ -1,22 +1,8 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.23; -import './BNum.sol'; - -// Highly opinionated token implementation - -interface IERC20 { - event Approval(address indexed src, address indexed dst, uint256 amt); - event Transfer(address indexed src, address indexed dst, uint256 amt); - - function totalSupply() external view returns (uint256); - function balanceOf(address whom) external view returns (uint256); - function allowance(address src, address dst) external view returns (uint256); - - function approve(address dst, uint256 amt) external returns (bool); - function transfer(address dst, uint256 amt) external returns (bool); - function transferFrom(address src, address dst, uint256 amt) external returns (bool); -} +import {BNum} from './BNum.sol'; +import {IERC20} from 'forge-std/interfaces/IERC20.sol'; abstract contract BTokenBase is BNum, IERC20 { mapping(address => uint256) internal _balance; @@ -57,30 +43,6 @@ contract BToken is BTokenBase { string internal _symbol = 'BPT'; uint8 internal _decimals = 18; - function name() public view returns (string memory) { - return _name; - } - - function symbol() public view returns (string memory) { - return _symbol; - } - - function decimals() public view returns (uint8) { - return _decimals; - } - - function allowance(address src, address dst) external view override returns (uint256) { - return _allowance[src][dst]; - } - - function balanceOf(address whom) external view override returns (uint256) { - return _balance[whom]; - } - - function totalSupply() public view override returns (uint256) { - return _totalSupply; - } - function approve(address dst, uint256 amt) external override returns (bool) { _allowance[msg.sender][dst] = amt; emit Approval(msg.sender, dst, amt); @@ -118,4 +80,28 @@ contract BToken is BTokenBase { } return true; } + + function allowance(address src, address dst) external view override returns (uint256) { + return _allowance[src][dst]; + } + + function balanceOf(address whom) external view override returns (uint256) { + return _balance[whom]; + } + + function totalSupply() public view override returns (uint256) { + return _totalSupply; + } + + function name() public view returns (string memory) { + return _name; + } + + function symbol() public view returns (string memory) { + return _symbol; + } + + function decimals() public view returns (uint8) { + return _decimals; + } } diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index b2d2e21f..009342f6 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -1068,7 +1068,8 @@ contract BPool_Unit_Unbind is BasePoolTest { vm.assume(_fuzz.balance >= MIN_BALANCE); vm.assume(_fuzz.totalWeight >= MIN_WEIGHT); vm.assume(_fuzz.totalWeight <= MAX_TOTAL_WEIGHT - MIN_WEIGHT); - _fuzz.previousTokensAmount = bound(_fuzz.previousTokensAmount, 1, MAX_BOUND_TOKENS); // The token to unbind will be included inside the array + // The token to unbind will be included inside the array + _fuzz.previousTokensAmount = bound(_fuzz.previousTokensAmount, 1, MAX_BOUND_TOKENS); _fuzz.tokenIndex = bound(_fuzz.tokenIndex, 0, _fuzz.previousTokensAmount - 1); _fuzz.denorm = bound(_fuzz.denorm, MIN_WEIGHT, _fuzz.totalWeight); _fuzz.previousTokens = new address[](_fuzz.previousTokensAmount);