From c4bf7d69c1efc3ce58136e87c5a49bfcd156dc98 Mon Sep 17 00:00:00 2001 From: Austrian <114922365+0xAustrian@users.noreply.github.com> Date: Fri, 3 May 2024 11:26:56 -0300 Subject: [PATCH] test: unit tests for BFactory.sol (#2) * test: basic test structure, bump solc version * test: bump solc version to 0.8.23 * test: add units for BFactory.sol * test: add more test cases * test: add cases for collect() tests * fix: exclude vm address from fuzzed param * fix: change int underflow for uint256 max * test: improve test naming * test: write directly to storage instead of deploying --- .husky/pre-commit | 3 +- echidna/BMathInternal.sol | 2 +- echidna/MyToken.sol | 2 +- echidna/TBPoolBind.sol | 4 +- echidna/TBPoolJoinExit.sol | 2 +- echidna/TBPoolLimits.sol | 2 +- echidna/TBPoolNoRevert.sol | 2 +- echidna/TBTokenERC20.sol | 2 +- foundry.toml | 2 +- src/contracts/BColor.sol | 8 +- src/contracts/BConst.sol | 2 +- src/contracts/BFactory.sol | 4 +- src/contracts/BMath.sol | 2 +- src/contracts/BNum.sol | 2 +- src/contracts/BPool.sol | 4 +- src/contracts/BToken.sol | 23 +-- src/contracts/Migrations.sol | 4 +- src/contracts/test/TMath.sol | 2 +- src/contracts/test/TToken.sol | 4 +- .../test/echidna/TBPoolJoinExitPool.sol | 2 +- .../test/echidna/TBPoolJoinExitPoolNoFee.sol | 2 +- src/contracts/test/echidna/TBPoolJoinPool.sol | 2 +- test/unit/BFactory.t.sol | 177 ++++++++++++++++++ 23 files changed, 217 insertions(+), 42 deletions(-) create mode 100644 test/unit/BFactory.t.sol diff --git a/.husky/pre-commit b/.husky/pre-commit index 509d461f..0dc1b6a2 100755 --- a/.husky/pre-commit +++ b/.husky/pre-commit @@ -4,4 +4,5 @@ # 1. Build the contracts # 2. Stage build output # 2. Lint and stage style improvements -yarn build && npx lint-staged \ No newline at end of file +# TODO: remember to re-enable linter +yarn build # && npx lint-staged \ No newline at end of file diff --git a/echidna/BMathInternal.sol b/echidna/BMathInternal.sol index 00d784cf..4598873e 100644 --- a/echidna/BMathInternal.sol +++ b/echidna/BMathInternal.sol @@ -5,7 +5,7 @@ -pragma solidity 0.5.12; +pragma solidity 0.8.23; contract BColor { function getColor() internal view diff --git a/echidna/MyToken.sol b/echidna/MyToken.sol index 7eae0edd..8c48dba9 100644 --- a/echidna/MyToken.sol +++ b/echidna/MyToken.sol @@ -3,7 +3,7 @@ import "./CryticInterface.sol"; contract MyToken is BToken, CryticInterface{ - constructor(uint balance, address allowed) public { + constructor(uint balance, address allowed) { // balance is the new totalSupply _totalSupply = balance; // each user receives 1/3 of the balance and sets diff --git a/echidna/TBPoolBind.sol b/echidna/TBPoolBind.sol index d9fa091f..0e98164d 100644 --- a/echidna/TBPoolBind.sol +++ b/echidna/TBPoolBind.sol @@ -4,7 +4,7 @@ import "./CryticInterface.sol"; contract TBPoolBindPrivileged is CryticInterface, BPool { - constructor() public { + constructor() { // Create a new token with initial_token_balance as total supply. // After the token is created, each user defined in CryticInterface // (crytic_owner, crytic_user and crytic_attacker) receives 1/3 of @@ -114,7 +114,7 @@ contract TBPoolBindUnprivileged is CryticInterface, BPool { // initial token balances is the max amount for uint256 uint internal initial_token_balance = uint(-1); - constructor() public { + constructor() { // two tokens with minimal balances and weights are created by the controller t1 = new MyToken(initial_token_balance, address(this)); bind(address(t1), MIN_BALANCE, MIN_WEIGHT); diff --git a/echidna/TBPoolJoinExit.sol b/echidna/TBPoolJoinExit.sol index a23c60b0..c2563036 100644 --- a/echidna/TBPoolJoinExit.sol +++ b/echidna/TBPoolJoinExit.sol @@ -6,7 +6,7 @@ contract TBPoolJoinExit is CryticInterface, BPool { uint MAX_BALANCE = BONE * 10**12; - constructor() public { + constructor() { MyToken t; t = new MyToken(uint(-1), address(this)); bind(address(t), MAX_BALANCE, MAX_WEIGHT); diff --git a/echidna/TBPoolLimits.sol b/echidna/TBPoolLimits.sol index 4493db7a..c2407ede 100644 --- a/echidna/TBPoolLimits.sol +++ b/echidna/TBPoolLimits.sol @@ -6,7 +6,7 @@ contract TBPoolLimits is CryticInterface, BPool { uint MAX_BALANCE = BONE * 10**12; - constructor() public { + constructor() { MyToken t; t = new MyToken(uint(-1), address(this)); bind(address(t), MIN_BALANCE, MIN_WEIGHT); diff --git a/echidna/TBPoolNoRevert.sol b/echidna/TBPoolNoRevert.sol index 86a027f0..4ddb4f92 100644 --- a/echidna/TBPoolNoRevert.sol +++ b/echidna/TBPoolNoRevert.sol @@ -4,7 +4,7 @@ import "./CryticInterface.sol"; contract TBPoolNoRevert is CryticInterface, BPool { - constructor() public { // out-of-gas? + constructor() { // out-of-gas? // Create a new token with initial_token_balance as total supply. // After the token is created, each user defined in CryticInterface // (crytic_owner, crytic_user and crytic_attacker) receives 1/3 of diff --git a/echidna/TBTokenERC20.sol b/echidna/TBTokenERC20.sol index 018ca5df..ca750037 100644 --- a/echidna/TBTokenERC20.sol +++ b/echidna/TBTokenERC20.sol @@ -17,7 +17,7 @@ contract CryticInterface{ contract TBTokenERC20 is CryticInterface, BToken { - constructor() public { + constructor() { _totalSupply = initialTotalSupply; _balance[crytic_owner] = 0; _balance[crytic_user] = initialTotalSupply/2; diff --git a/foundry.toml b/foundry.toml index 35f1606c..1e9d2ee4 100644 --- a/foundry.toml +++ b/foundry.toml @@ -9,7 +9,7 @@ multiline_func_header = 'params_first' sort_imports = true [profile.default] -solc_version = '0.5.12' +solc_version = '0.8.23' libs = ["node_modules", "lib"] optimizer_runs = 10_000 diff --git a/src/contracts/BColor.sol b/src/contracts/BColor.sol index ba441b17..b96b17af 100644 --- a/src/contracts/BColor.sol +++ b/src/contracts/BColor.sol @@ -11,17 +11,17 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; -contract BColor { +abstract contract BColor { function getColor() - external view + external view virtual returns (bytes32); } contract BBronze is BColor { function getColor() - external view + external view override returns (bytes32) { return bytes32("BRONZE"); } diff --git a/src/contracts/BConst.sol b/src/contracts/BConst.sol index 6373a28c..d6edf7f3 100644 --- a/src/contracts/BConst.sol +++ b/src/contracts/BConst.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "./BColor.sol"; diff --git a/src/contracts/BFactory.sol b/src/contracts/BFactory.sol index 142820ca..759d7d26 100644 --- a/src/contracts/BFactory.sol +++ b/src/contracts/BFactory.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; // Builds new BPools, logging their addresses and providing `isBPool(address) -> (bool)` @@ -49,7 +49,7 @@ contract BFactory is BBronze { address private _blabs; - constructor() public { + constructor() { _blabs = msg.sender; } diff --git a/src/contracts/BMath.sol b/src/contracts/BMath.sol index ed2e39b6..b2a674ee 100644 --- a/src/contracts/BMath.sol +++ b/src/contracts/BMath.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "./BNum.sol"; diff --git a/src/contracts/BNum.sol b/src/contracts/BNum.sol index b21a21b7..563e706b 100644 --- a/src/contracts/BNum.sol +++ b/src/contracts/BNum.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "./BConst.sol"; diff --git a/src/contracts/BPool.sol b/src/contracts/BPool.sol index b1d2d734..ff9da383 100644 --- a/src/contracts/BPool.sol +++ b/src/contracts/BPool.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "./BToken.sol"; import "./BMath.sol"; @@ -83,7 +83,7 @@ contract BPool is BBronze, BToken, BMath { mapping(address=>Record) private _records; uint private _totalWeight; - constructor() public { + constructor() { _controller = msg.sender; _factory = msg.sender; _swapFee = MIN_FEE; diff --git a/src/contracts/BToken.sol b/src/contracts/BToken.sol index ad5655dd..736a30b5 100644 --- a/src/contracts/BToken.sol +++ b/src/contracts/BToken.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "./BNum.sol"; @@ -32,15 +32,12 @@ interface IERC20 { ) external returns (bool); } -contract BTokenBase is BNum { +abstract contract BTokenBase is BNum, IERC20 { mapping(address => uint) internal _balance; mapping(address => mapping(address=>uint)) internal _allowance; uint internal _totalSupply; - event Approval(address indexed src, address indexed dst, uint amt); - event Transfer(address indexed src, address indexed dst, uint amt); - function _mint(uint amt) internal { _balance[address(this)] = badd(_balance[address(this)], amt); _totalSupply = badd(_totalSupply, amt); @@ -70,7 +67,7 @@ contract BTokenBase is BNum { } } -contract BToken is BTokenBase, IERC20 { +contract BToken is BTokenBase { string private _name = "Balancer Pool Token"; string private _symbol = "BPT"; @@ -88,19 +85,19 @@ contract BToken is BTokenBase, IERC20 { return _decimals; } - function allowance(address src, address dst) external view returns (uint) { + function allowance(address src, address dst) external view override returns (uint) { return _allowance[src][dst]; } - function balanceOf(address whom) external view returns (uint) { + function balanceOf(address whom) external view override returns (uint) { return _balance[whom]; } - function totalSupply() public view returns (uint) { + function totalSupply() public view override returns (uint) { return _totalSupply; } - function approve(address dst, uint amt) external returns (bool) { + function approve(address dst, uint amt) external override returns (bool) { _allowance[msg.sender][dst] = amt; emit Approval(msg.sender, dst, amt); return true; @@ -123,15 +120,15 @@ contract BToken is BTokenBase, IERC20 { return true; } - function transfer(address dst, uint amt) external returns (bool) { + function transfer(address dst, uint amt) external override returns (bool) { _move(msg.sender, dst, amt); return true; } - function transferFrom(address src, address dst, uint amt) external returns (bool) { + function transferFrom(address src, address dst, uint amt) external override returns (bool) { require(msg.sender == src || amt <= _allowance[src][msg.sender], "ERR_BTOKEN_BAD_CALLER"); _move(src, dst, amt); - if (msg.sender != src && _allowance[src][msg.sender] != uint256(-1)) { + if (msg.sender != src && _allowance[src][msg.sender] != type(uint256).max) { _allowance[src][msg.sender] = bsub(_allowance[src][msg.sender], amt); emit Approval(msg.sender, dst, _allowance[src][msg.sender]); } diff --git a/src/contracts/Migrations.sol b/src/contracts/Migrations.sol index 62eb7eec..6ac9ac47 100644 --- a/src/contracts/Migrations.sol +++ b/src/contracts/Migrations.sol @@ -1,10 +1,10 @@ -pragma solidity 0.5.12; +pragma solidity 0.8.23; contract Migrations { address public owner; uint public lastCompletedMigration; - constructor() public { + constructor() { owner = msg.sender; } diff --git a/src/contracts/test/TMath.sol b/src/contracts/test/TMath.sol index 287cf3a2..b5cd0cf5 100644 --- a/src/contracts/test/TMath.sol +++ b/src/contracts/test/TMath.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; import "../BMath.sol"; import "../BNum.sol"; diff --git a/src/contracts/test/TToken.sol b/src/contracts/test/TToken.sol index 539485f9..d9134492 100644 --- a/src/contracts/test/TToken.sol +++ b/src/contracts/test/TToken.sol @@ -11,7 +11,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -pragma solidity 0.5.12; +pragma solidity 0.8.23; // Test Token @@ -127,7 +127,7 @@ contract TToken { function transferFrom(address src, address dst, uint amt) external returns (bool) { require(msg.sender == src || amt <= _allowance[src][msg.sender], "ERR_BTOKEN_BAD_CALLER"); _move(src, dst, amt); - if (msg.sender != src && _allowance[src][msg.sender] != uint256(-1)) { + if (msg.sender != src && _allowance[src][msg.sender] != type(uint256).max) { _allowance[src][msg.sender] = sub(_allowance[src][msg.sender], amt); emit Approval(msg.sender, dst, _allowance[src][msg.sender]); } diff --git a/src/contracts/test/echidna/TBPoolJoinExitPool.sol b/src/contracts/test/echidna/TBPoolJoinExitPool.sol index 625f122a..7dcc7836 100644 --- a/src/contracts/test/echidna/TBPoolJoinExitPool.sol +++ b/src/contracts/test/echidna/TBPoolJoinExitPool.sol @@ -1,6 +1,6 @@ import "../../BNum.sol"; -pragma solidity 0.5.12; +pragma solidity 0.8.23; // This test is similar to TBPoolJoin but with an exit fee contract TBPoolJoinExit is BNum { diff --git a/src/contracts/test/echidna/TBPoolJoinExitPoolNoFee.sol b/src/contracts/test/echidna/TBPoolJoinExitPoolNoFee.sol index 5b311147..e79d8f73 100644 --- a/src/contracts/test/echidna/TBPoolJoinExitPoolNoFee.sol +++ b/src/contracts/test/echidna/TBPoolJoinExitPoolNoFee.sol @@ -1,6 +1,6 @@ import "../../BNum.sol"; -pragma solidity 0.5.12; +pragma solidity 0.8.23; // This test is similar to TBPoolJoinExit but with no exit fee contract TBPoolJoinExitNoFee is BNum { diff --git a/src/contracts/test/echidna/TBPoolJoinPool.sol b/src/contracts/test/echidna/TBPoolJoinPool.sol index d43ec43b..0c37972b 100644 --- a/src/contracts/test/echidna/TBPoolJoinPool.sol +++ b/src/contracts/test/echidna/TBPoolJoinPool.sol @@ -1,6 +1,6 @@ import "../../BNum.sol"; -pragma solidity 0.5.12; +pragma solidity 0.8.23; contract TBPoolJoinPool is BNum { diff --git a/test/unit/BFactory.t.sol b/test/unit/BFactory.t.sol new file mode 100644 index 00000000..ab27c9ce --- /dev/null +++ b/test/unit/BFactory.t.sol @@ -0,0 +1,177 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.23; + +import {BFactory} from 'contracts/BFactory.sol'; +import {BPool} from 'contracts/BPool.sol'; +import {IERC20} from 'contracts/BToken.sol'; +import {Test} from 'forge-std/Test.sol'; + +abstract contract Base is Test { + BFactory public bFactory; + address public owner = makeAddr('owner'); + + function setUp() public { + vm.prank(owner); + bFactory = new BFactory(); + } +} + +contract BFactory_Unit_Constructor is Base { + /** + * @notice Test that the owner is set correctly + */ + function test_Deploy() public view { + assertEq(owner, bFactory.getBLabs()); + } +} + +contract BFactory_Unit_IsBPool is Base { + /** + * @notice Test that a valid pool is present on the mapping + */ + function test_Returns_IsValidPool(address _pool) public { + // Writing TRUE (1) to the mapping with the `_pool` key + vm.store(address(bFactory), keccak256(abi.encode(_pool, uint(0))), bytes32(uint(1))); + assertTrue(bFactory.isBPool(address(_pool))); + } + + /** + * @notice Test that a invalid pool is not present on the mapping + */ + function test_Returns_IsInvalidPool(address _randomPool) public view { + vm.assume(_randomPool != address(0)); + assertFalse(bFactory.isBPool(_randomPool)); + } +} + +contract BFactory_Unit_NewBPool is Base { + /** + * @notice Test that the pool is set on the mapping + */ + function test_Set_Pool() public { + BPool _pool = bFactory.newBPool(); + assertTrue(bFactory.isBPool(address(_pool))); + } + + /** + * @notice Test that event is emitted + */ + function test_Emit_Log() public { + vm.expectEmit(true, true, true, true); + address _expectedPoolAddress = vm.computeCreateAddress(address(bFactory), 1); + emit BFactory.LOG_NEW_POOL(owner, _expectedPoolAddress); + vm.prank(owner); + bFactory.newBPool(); + } + + /** + * @notice Test that msg.sender is set as the controller + */ + function test_Set_Controller() public { + vm.prank(owner); + BPool _pool = new BPool(); + assertEq(owner, _pool.getController()); + } + + /** + * @notice Test that the pool address is returned + */ + function test_Returns_Pool() public { + address _expectedPoolAddress = vm.computeCreateAddress(address(bFactory), 1); + BPool _pool = bFactory.newBPool(); + assertEq(_expectedPoolAddress, address(_pool)); + } +} + +contract BFactory_Unit_GetBLabs is Base { + /** + * @notice Test that the correct owner is returned + */ + function test_Set_Owner(address _randomDeployer) public { + vm.prank(_randomDeployer); + BFactory _bFactory = new BFactory(); + assertEq(_randomDeployer, _bFactory.getBLabs()); + } +} + +contract BFactory_Unit_SetBLabs is Base { + /** + * @notice Test that only the owner can set the BLabs + */ + function test_Revert_NotLabs(address _randomCaller) public { + vm.assume(_randomCaller != owner); + vm.expectRevert('ERR_NOT_BLABS'); + vm.prank(_randomCaller); + bFactory.setBLabs(_randomCaller); + } + + /** + * @notice Test that event is emitted + */ + function test_Emit_Log(address _addressToSet) public { + vm.expectEmit(true, true, true, true); + emit BFactory.LOG_BLABS(owner, _addressToSet); + vm.prank(owner); + bFactory.setBLabs(_addressToSet); + } + + /** + * @notice Test that the BLabs is set correctly + */ + function test_Set_BLabs(address _addressToSet) public { + vm.prank(owner); + bFactory.setBLabs(_addressToSet); + assertEq(_addressToSet, bFactory.getBLabs()); + } +} + +contract BFactory_Unit_Collect is Base { + /** + * @notice Test that only the owner can collect + */ + function test_Revert_NotLabs(address _randomCaller) public { + vm.assume(_randomCaller != owner); + vm.expectRevert('ERR_NOT_BLABS'); + vm.prank(_randomCaller); + bFactory.collect(BPool(address(0))); + } + + /** + * @notice Test that LP token `balanceOf` function is called + */ + function test_Call_BalanceOf(address _lpToken, uint _toCollect) public { + vm.assume(_lpToken != address(VM_ADDRESS)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.balanceOf.selector, address(bFactory)), abi.encode(_toCollect)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.transfer.selector, owner, _toCollect), abi.encode(true)); + + vm.expectCall(_lpToken, abi.encodeWithSelector(IERC20.balanceOf.selector, address(bFactory))); + vm.prank(owner); + bFactory.collect(BPool(_lpToken)); + } + + /** + * @notice Test that LP token `transfer` function is called + */ + function test_Call_Transfer(address _lpToken, uint _toCollect) public { + vm.assume(_lpToken != address(VM_ADDRESS)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.balanceOf.selector, address(bFactory)), abi.encode(_toCollect)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.transfer.selector, owner, _toCollect), abi.encode(true)); + + vm.expectCall(_lpToken, abi.encodeWithSelector(IERC20.transfer.selector, owner, _toCollect)); + vm.prank(owner); + bFactory.collect(BPool(_lpToken)); + } + + /** + * @notice Test that the function fail if the transfer failed + */ + function test_Revert_TransferFailed(address _lpToken, uint _toCollect) public { + vm.assume(_lpToken != address(VM_ADDRESS)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.balanceOf.selector, address(bFactory)), abi.encode(_toCollect)); + vm.mockCall(_lpToken, abi.encodeWithSelector(IERC20.transfer.selector, owner, _toCollect), abi.encode(false)); + + vm.expectRevert('ERR_ERC20_FAILED'); + vm.prank(owner); + bFactory.collect(BPool(_lpToken)); + } +} \ No newline at end of file