diff --git a/contracts/common/IERC20MintableBurnable.sol b/contracts/common/IERC20MintableBurnable.sol new file mode 100644 index 0000000..1c4be41 --- /dev/null +++ b/contracts/common/IERC20MintableBurnable.sol @@ -0,0 +1,81 @@ +pragma solidity ^0.8.0; +//TODO: check if this is latest impl of openzeppelin or any other impl + +/** + * @dev Interface of the ERC20 standard as defined in the EIP. Does not include + * the optional functions; to access them see {ERC20Detailed}. + */ +interface IERC20MintableBurnable { + /** + * @dev Returns the amount of tokens in existence. + */ + function totalSupply() external view returns (uint256); + + /** + * @dev Returns the amount of tokens owned by `account`. + */ + function balanceOf(address account) external view returns (uint256); + + /** + * @dev Moves `amount` tokens from the caller's account to `recipient`. + * + * Returns a boolean value indicating whether the operation succeeded. + * + * Emits a {Transfer} event. + */ + function transfer(address recipient, uint256 amount) external returns (bool); + + function mint(address account, uint256 amount) external; + + function burn(uint256 amount) external; + + /** + * @dev Returns the remaining number of tokens that `spender` will be + * allowed to spend on behalf of `owner` through {transferFrom}. This is + * zero by default. + * + * This value changes when {approve} or {transferFrom} are called. + */ + function allowance(address owner, address spender) external view returns (uint256); + + /** + * @dev Sets `amount` as the allowance of `spender` over the caller's tokens. + * + * Returns a boolean value indicating whether the operation succeeded. + * + * IMPORTANT: Beware that changing an allowance with this method brings the risk + * that someone may use both the old and the new allowance by unfortunate + * transaction ordering. One possible solution to mitigate this race + * condition is to first reduce the spender's allowance to 0 and set the + * desired value afterwards: + * https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729 + * + * Emits an {Approval} event. + */ + function approve(address spender, uint256 amount) external returns (bool); + + /** + * @dev Moves `amount` tokens from `sender` to `recipient` using the + * allowance mechanism. `amount` is then deducted from the caller's + * allowance. + * + * Returns a boolean value indicating whether the operation succeeded. + * + * Emits a {Transfer} event. + */ + function transferFrom(address sender, address recipient, uint256 amount) external returns (bool); + + /** + * @dev Emitted when `value` tokens are moved from one account (`from`) to + * another (`to`). + * + * Note that `value` may be zero. + */ + event Transfer(address indexed from, address indexed to, uint256 value); + + /** + * @dev Emitted when the allowance of a `spender` for an `owner` is set by + * a call to {approve}. `value` is the new allowance. + */ + event Approval(address indexed owner, address indexed spender, uint256 value); +} diff --git a/contracts/common/SafeERC20.sol b/contracts/common/SafeERC20.sol new file mode 100644 index 0000000..2286277 --- /dev/null +++ b/contracts/common/SafeERC20.sol @@ -0,0 +1,83 @@ +pragma solidity ^0.8.0; + +import { IERC20MintableBurnable as IERC20 } from "contracts/common/IERC20MintableBurnable.sol"; +import { Address } from "openzeppelin-contracts-next/contracts/utils/Address.sol"; +//TODO: merge with lattest impl of openzeppelin +/** + * @title SafeERC20 + * @dev Wrappers around ERC20 operations that throw on failure (when the token + * contract returns false). Tokens that return no value (and instead revert or + * throw on failure) are also supported, non-reverting calls are assumed to be + * successful. + * To use this library you can add a `using SafeERC20 for ERC20;` statement to your contract, + * which allows you to call the safe operations as `token.safeTransfer(...)`, etc. + */ +library SafeERC20 { + using Address for address; + + function safeTransfer(IERC20 token, address to, uint256 value) internal { + callOptionalReturn(token, abi.encodeWithSelector(token.transfer.selector, to, value)); + } + + function safeTransferFrom(IERC20 token, address from, address to, uint256 value) internal { + callOptionalReturn(token, abi.encodeWithSelector(token.transferFrom.selector, from, to, value)); + } + + function safeMint(IERC20 token, address account, uint256 amount) internal { + callOptionalReturn(token, abi.encodeWithSelector(token.mint.selector, account, amount)); + } + + function safeBurn(IERC20 token, uint256 amount) internal { + callOptionalReturn(token, abi.encodeWithSelector(token.burn.selector, amount)); + } + + function safeApprove(IERC20 token, address spender, uint256 value) internal { + // safeApprove should only be called when setting an initial allowance, + // or when resetting it to zero. To increase and decrease it, use + // 'safeIncreaseAllowance' and 'safeDecreaseAllowance' + // solhint-disable-next-line max-line-length + require( + (value == 0) || (token.allowance(address(this), spender) == 0), + "SafeERC20: approve from non-zero to non-zero allowance" + ); + callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value)); + } + + function safeIncreaseAllowance(IERC20 token, address spender, uint256 value) internal { + uint256 newAllowance = token.allowance(address(this), spender) + value; + callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance)); + } + + function safeDecreaseAllowance(IERC20 token, address spender, uint256 value) internal { + uint256 newAllowance = token.allowance(address(this), spender) - value; + callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, newAllowance)); + } + + /** + * @dev Imitates a Solidity high-level call (i.e. a regular function call to a contract), relaxing the requirement + * on the return value: the return value is optional (but if data is returned, it must not be false). + * @param token The token targeted by the call. + * @param data The call data (encoded using abi.encode or one of its variants). + */ + function callOptionalReturn(IERC20 token, bytes memory data) private { + // We need to perform a low level call here, to bypass Solidity's return data size checking mechanism, since + // we're implementing it ourselves. + + // A Solidity high level call has three parts: + // 1. The target address is checked to verify it contains contract code + // 2. The call itself is made, and success asserted + // 3. The return value is decoded, which in turn checks the size of the returned data. + // solhint-disable-next-line max-line-length + require(address(token).isContract(), "SafeERC20: call to non-contract"); + + // solhint-disable-next-line avoid-low-level-calls + (bool success, bytes memory returndata) = address(token).call(data); + require(success, "SafeERC20: low-level call failed"); + + if (returndata.length > 0) { + // Return data is optional + // solhint-disable-next-line max-line-length + require(abi.decode(returndata, (bool)), "SafeERC20: ERC20 operation did not succeed"); + } + } +} diff --git a/contracts/import.sol b/contracts/import.sol index e74ef42..8614396 100644 --- a/contracts/import.sol +++ b/contracts/import.sol @@ -12,4 +12,3 @@ import "celo/contracts/common/Freezer.sol"; import "celo/contracts/stability/SortedOracles.sol"; import "test/utils/harnesses/WithThresholdHarness.sol"; import "test/utils/harnesses/WithCooldownHarness.sol"; -import "test/utils/harnesses/TradingLimitsHarness.sol"; diff --git a/contracts/interfaces/IBroker.sol b/contracts/interfaces/IBroker.sol index 675713b..1df8193 100644 --- a/contracts/interfaces/IBroker.sol +++ b/contracts/interfaces/IBroker.sol @@ -121,34 +121,39 @@ interface IBroker { /** * @notice Allows the contract to be upgradable via the proxy. * @param _exchangeProviders The addresses of the ExchangeProvider contracts. - * @param _reserve The address of the Reserve contract. + * @param _reserves The address of the Reserve contract. */ - function initialize(address[] calldata _exchangeProviders, address _reserve) external; + function initialize(address[] calldata _exchangeProviders, address[] calldata _reserves) external; - /// @notice IOwnable: - function transferOwnership(address newOwner) external; + //TODO: Bogdan added these to the interface but when upgrading to 0.8.18, + // This is causing a compilation error. because they are defined in Ownable.sol as well. + // only way of fixing this is to remove them from the interface. or have the explicit + // implementation in the contract and add a override(IBroker, Ownable) to the functions. + + //function renounceOwnership() external; - function renounceOwnership() external; + //function owner() external view returns (address); - function owner() external view returns (address); + /// @notice IOwnable: + //function transferOwnership(address newOwner) external; /// @notice Getters: - function reserve() external view returns (address); + //function reserve() external view returns (address); function isExchangeProvider(address exchangeProvider) external view returns (bool); /// @notice Setters: - function addExchangeProvider(address exchangeProvider) external returns (uint256 index); + function addExchangeProvider(address exchangeProvider, address reserve) external returns (uint256 index); function removeExchangeProvider(address exchangeProvider, uint256 index) external; - function setReserve(address _reserve) external; + function setReserves(address[] calldata _exchangeProviders, address[] calldata _reserves) external; function configureTradingLimit(bytes32 exchangeId, address token, ITradingLimits.Config calldata config) external; - function tradingLimitsConfig(bytes32 id) external view returns (ITradingLimits.Config memory); + // function tradingLimitsConfig(bytes32 id) external view returns (ITradingLimits.Config memory); - function tradingLimitsState(bytes32 id) external view returns (ITradingLimits.State memory); + // function tradingLimitsState(bytes32 id) external view returns (ITradingLimits.State memory); function exchangeProviders(uint256 i) external view returns (address); } diff --git a/contracts/interfaces/IBrokerAdmin.sol b/contracts/interfaces/IBrokerAdmin.sol index d0aba86..6cac3dd 100644 --- a/contracts/interfaces/IBrokerAdmin.sol +++ b/contracts/interfaces/IBrokerAdmin.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.5.13; +pragma solidity >=0.5.13 <0.8.19; /* * @title Broker Admin Interface @@ -38,11 +38,12 @@ interface IBrokerAdmin { * @param exchangeProvider The address of the ExchangeProvider to add. * @return index The index where the ExchangeProvider was inserted. */ - function addExchangeProvider(address exchangeProvider) external returns (uint256 index); + function addExchangeProvider(address exchangeProvider, address reserve) external returns (uint256 index); /** - * @notice Set the Mento reserve address. - * @param reserve The Mento reserve address. + * @notice Set the reserves for the exchange providers. + * @param _exchangeProviders The addresses of the ExchangeProvider contracts. + * @param _reserves The addresses of the Reserve contracts. */ - function setReserve(address reserve) external; + function setReserves(address[] calldata _exchangeProviders, address[] calldata _reserves) external; } diff --git a/contracts/libraries/TradingLimits.sol b/contracts/libraries/TradingLimits.sol index 66ad2cf..14e5954 100644 --- a/contracts/libraries/TradingLimits.sol +++ b/contracts/libraries/TradingLimits.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.5.13; +pragma solidity 0.8.18; pragma experimental ABIEncoderV2; import { ITradingLimits } from "contracts/interfaces/ITradingLimits.sol"; @@ -44,7 +44,7 @@ library TradingLimits { uint8 private constant L0 = 1; // 0b001 Limit0 uint8 private constant L1 = 2; // 0b010 Limit1 uint8 private constant LG = 4; // 0b100 LimitGlobal - int48 private constant MAX_INT48 = int48(uint48(-1) / 2); + int48 private constant MAX_INT48 = type(int48).max; /** * @notice Validate a trading limit configuration. @@ -129,7 +129,11 @@ library TradingLimits { ) internal view returns (ITradingLimits.State memory) { int256 _deltaFlowUnits = _deltaFlow / int256((10 ** uint256(decimals))); require(_deltaFlowUnits <= MAX_INT48, "dFlow too large"); - int48 deltaFlowUnits = _deltaFlowUnits == 0 ? 1 : int48(_deltaFlowUnits); + + int48 deltaFlowUnits = int48(_deltaFlowUnits); + if (deltaFlowUnits == 0) { + deltaFlowUnits = _deltaFlow > 0 ? int48(1) : int48(-1); + } if (config.flags & L0 > 0) { if (block.timestamp > self.lastUpdated0 + config.timestep0) { diff --git a/contracts/swap/Broker.sol b/contracts/swap/Broker.sol index 234172c..d097505 100644 --- a/contracts/swap/Broker.sol +++ b/contracts/swap/Broker.sol @@ -1,11 +1,10 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.5.13; +pragma solidity 0.8.18; pragma experimental ABIEncoderV2; -import { Ownable } from "openzeppelin-solidity/contracts/ownership/Ownable.sol"; -import { SafeERC20 } from "openzeppelin-solidity/contracts/token/ERC20/SafeERC20.sol"; -import { IERC20 } from "openzeppelin-solidity/contracts/token/ERC20/IERC20.sol"; -import { SafeMath } from "openzeppelin-solidity/contracts/math/SafeMath.sol"; +import { Ownable } from "openzeppelin-contracts-next/contracts/access/Ownable.sol"; +import { SafeERC20 } from "contracts/common/SafeERC20.sol"; +import { IERC20MintableBurnable as IERC20 } from "contracts/common/IERC20MintableBurnable.sol"; import { IExchangeProvider } from "../interfaces/IExchangeProvider.sol"; import { IBroker } from "../interfaces/IBroker.sol"; @@ -16,7 +15,8 @@ import { ITradingLimits } from "../interfaces/ITradingLimits.sol"; import { TradingLimits } from "../libraries/TradingLimits.sol"; import { Initializable } from "celo/contracts/common/Initializable.sol"; -import { ReentrancyGuard } from "celo/contracts/common/libraries/ReentrancyGuard.sol"; +// TODO: ensure we can use latest impl of openzeppelin +import { ReentrancyGuard } from "openzeppelin-contracts-next/contracts/security/ReentrancyGuard.sol"; interface IERC20Metadata { /** @@ -33,7 +33,6 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar using TradingLimits for ITradingLimits.State; using TradingLimits for ITradingLimits.Config; using SafeERC20 for IERC20; - using SafeMath for uint256; /* ==================== State Variables ==================== */ @@ -43,29 +42,46 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar mapping(bytes32 => ITradingLimits.Config) public tradingLimitsConfig; // Address of the reserve. - IReserve public reserve; + uint256 public __deprecated0; // prev: IReserve public reserve; - uint256 private constant MAX_INT256 = uint256(-1) / 2; + uint256 private constant MAX_INT256 = uint256(type(int256).max); + mapping(address => address) public exchangeReserve; /* ==================== Constructor ==================== */ /** * @notice Sets initialized == true on implementation contracts. * @param test Set to true to skip implementation initialization. */ - constructor(bool test) public Initializable(test) {} + constructor(bool test) Initializable(test) {} /** * @notice Allows the contract to be upgradable via the proxy. * @param _exchangeProviders The addresses of the ExchangeProvider contracts. - * @param _reserve The address of the Reserve contract. + * @param _reserves The addresses of the Reserve contracts. */ - function initialize(address[] calldata _exchangeProviders, address _reserve) external initializer { + function initialize(address[] calldata _exchangeProviders, address[] calldata _reserves) external initializer { _transferOwnership(msg.sender); for (uint256 i = 0; i < _exchangeProviders.length; i++) { - addExchangeProvider(_exchangeProviders[i]); + addExchangeProvider(_exchangeProviders[i], _reserves[i]); + } + } + + /** + * @notice Set the reserves for the exchange providers. + * @param _exchangeProviders The addresses of the ExchangeProvider contracts. + * @param _reserves The addresses of the Reserve contracts. + */ + function setReserves( + address[] calldata _exchangeProviders, + address[] calldata _reserves + ) external override(IBroker, IBrokerAdmin) onlyOwner { + for (uint256 i = 0; i < _exchangeProviders.length; i++) { + require(isExchangeProvider[_exchangeProviders[i]], "ExchangeProvider does not exist"); + require(_reserves[i] != address(0), "Reserve address can't be 0"); + exchangeReserve[_exchangeProviders[i]] = _reserves[i]; + emit ReserveSet(_exchangeProviders[i], _reserves[i]); } - setReserve(_reserve); } /* ==================== Mutative Functions ==================== */ @@ -73,15 +89,22 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar /** * @notice Add an exchange provider to the list of providers. * @param exchangeProvider The address of the exchange provider to add. + * @param reserve The address of the reserve used by the exchange provider. * @return index The index of the newly added specified exchange provider. */ - function addExchangeProvider(address exchangeProvider) public onlyOwner returns (uint256 index) { + function addExchangeProvider( + address exchangeProvider, + address reserve + ) public override(IBroker, IBrokerAdmin) onlyOwner returns (uint256 index) { require(!isExchangeProvider[exchangeProvider], "ExchangeProvider already exists in the list"); require(exchangeProvider != address(0), "ExchangeProvider address can't be 0"); + require(reserve != address(0), "Reserve address can't be 0"); exchangeProviders.push(exchangeProvider); isExchangeProvider[exchangeProvider] = true; + exchangeReserve[exchangeProvider] = reserve; emit ExchangeProviderAdded(exchangeProvider); - index = exchangeProviders.length.sub(1); + emit ReserveSet(exchangeProvider, reserve); + index = exchangeProviders.length - 1; } /** @@ -89,24 +112,18 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar * @param exchangeProvider The address of the exchange provider to remove. * @param index The index of the exchange provider being removed. */ - function removeExchangeProvider(address exchangeProvider, uint256 index) public onlyOwner { + function removeExchangeProvider( + address exchangeProvider, + uint256 index + ) public override(IBroker, IBrokerAdmin) onlyOwner { require(exchangeProviders[index] == exchangeProvider, "index doesn't match provider"); - exchangeProviders[index] = exchangeProviders[exchangeProviders.length.sub(1)]; + exchangeProviders[index] = exchangeProviders[exchangeProviders.length - 1]; exchangeProviders.pop(); delete isExchangeProvider[exchangeProvider]; + delete exchangeReserve[exchangeProvider]; emit ExchangeProviderRemoved(exchangeProvider); } - /** - * @notice Set the Mento reserve address. - * @param _reserve The Mento reserve address. - */ - function setReserve(address _reserve) public onlyOwner { - require(_reserve != address(0), "Reserve address must be set"); - emit ReserveSet(_reserve, address(reserve)); - reserve = IReserve(_reserve); - } - /** * @notice Calculate the amount of tokenIn to be sold for a given amountOut of tokenOut * @param exchangeProvider the address of the exchange manager for the pair @@ -170,8 +187,10 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar amountOut = IExchangeProvider(exchangeProvider).swapIn(exchangeId, tokenIn, tokenOut, amountIn); require(amountOut >= amountOutMin, "amountOutMin not met"); guardTradingLimits(exchangeId, tokenIn, amountIn, tokenOut, amountOut); - transferIn(msg.sender, tokenIn, amountIn); - transferOut(msg.sender, tokenOut, amountOut); + + address reserve = exchangeReserve[exchangeProvider]; + transferIn(payable(msg.sender), tokenIn, amountIn, reserve); + transferOut(payable(msg.sender), tokenOut, amountOut, reserve); emit Swap(exchangeProvider, exchangeId, msg.sender, tokenIn, tokenOut, amountIn, amountOut); } @@ -198,8 +217,10 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar amountIn = IExchangeProvider(exchangeProvider).swapOut(exchangeId, tokenIn, tokenOut, amountOut); require(amountIn <= amountInMax, "amountInMax exceeded"); guardTradingLimits(exchangeId, tokenIn, amountIn, tokenOut, amountOut); - transferIn(msg.sender, tokenIn, amountIn); - transferOut(msg.sender, tokenOut, amountOut); + + address reserve = exchangeReserve[exchangeProvider]; + transferIn(payable(msg.sender), tokenIn, amountIn, reserve); + transferOut(payable(msg.sender), tokenOut, amountOut, reserve); emit Swap(exchangeProvider, exchangeId, msg.sender, tokenIn, tokenOut, amountIn, amountOut); } @@ -210,9 +231,8 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar * @return True if transaction succeeds. */ function burnStableTokens(address token, uint256 amount) public returns (bool) { - require(reserve.isStableAsset(token), "Token must be a reserve stable asset"); IERC20(token).safeTransferFrom(msg.sender, address(this), amount); - require(IStableTokenV2(token).burn(amount), "Burning of the stable asset failed"); + IERC20(token).safeBurn(amount); return true; } @@ -250,10 +270,12 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar * @param to The address receiving the asset. * @param token The asset to transfer. * @param amount The amount of `token` to be transferred. + * @param _reserve The address of the corresponding reserve. */ - function transferOut(address payable to, address token, uint256 amount) internal { + function transferOut(address payable to, address token, uint256 amount, address _reserve) internal { + IReserve reserve = IReserve(_reserve); if (reserve.isStableAsset(token)) { - require(IStableTokenV2(token).mint(to, amount), "Minting of the stable asset failed"); + IERC20(token).safeMint(to, amount); } else if (reserve.isCollateralAsset(token)) { require(reserve.transferExchangeCollateralAsset(token, to, amount), "Transfer of the collateral asset failed"); } else { @@ -268,11 +290,13 @@ contract Broker is IBroker, IBrokerAdmin, Initializable, Ownable, ReentrancyGuar * @param from The address to transfer the asset from. * @param token The asset to transfer. * @param amount The amount of `token` to be transferred. + * @param _reserve The address of the corresponding reserve. */ - function transferIn(address payable from, address token, uint256 amount) internal { + function transferIn(address payable from, address token, uint256 amount, address _reserve) internal { + IReserve reserve = IReserve(_reserve); if (reserve.isStableAsset(token)) { IERC20(token).safeTransferFrom(from, address(this), amount); - require(IStableTokenV2(token).burn(amount), "Burning of the stable asset failed"); + IERC20(token).safeBurn(amount); } else if (reserve.isCollateralAsset(token)) { IERC20(token).safeTransferFrom(from, address(reserve), amount); } else { diff --git a/test/fork/BaseForkTest.sol b/test/fork/BaseForkTest.sol index fe97e8a..ab4afbb 100644 --- a/test/fork/BaseForkTest.sol +++ b/test/fork/BaseForkTest.sol @@ -10,6 +10,7 @@ import { IRegistry } from "celo/contracts/common/interfaces/IRegistry.sol"; import { IBreakerBox } from "contracts/interfaces/IBreakerBox.sol"; import { IBroker } from "contracts/interfaces/IBroker.sol"; +import { Broker } from "contracts/swap/Broker.sol"; import { IReserve } from "contracts/interfaces/IReserve.sol"; import { ISortedOracles } from "contracts/interfaces/ISortedOracles.sol"; import { ITradingLimitsHarness } from "test/utils/harnesses/ITradingLimitsHarness.sol"; @@ -35,7 +36,7 @@ abstract contract BaseForkTest is Test { IRegistry public registry = IRegistry(CELO_REGISTRY_ADDRESS); address governance; - IBroker public broker; + Broker public broker; IBreakerBox public breakerBox; ISortedOracles public sortedOracles; IReserve public reserve; @@ -72,13 +73,13 @@ abstract contract BaseForkTest is Test { __CeloPrecompiles_init(); tradingLimits = ITradingLimitsHarness(deployCode("TradingLimitsHarness")); - broker = IBroker(lookup("Broker")); + broker = Broker(lookup("Broker")); sortedOracles = ISortedOracles(lookup("SortedOracles")); governance = lookup("Governance"); breakerBox = IBreakerBox(address(sortedOracles.breakerBox())); vm.label(address(breakerBox), "BreakerBox"); trader = makeAddr("trader"); - reserve = IReserve(broker.reserve()); + reserve = IReserve(lookup("Reserve")); /// @dev Hardcoded number of dependencies for each ratefeed. /// Should be updated when they change, there is a test that will diff --git a/test/fork/ChainForkTest.sol b/test/fork/ChainForkTest.sol index c6f4c32..6f89c95 100644 --- a/test/fork/ChainForkTest.sol +++ b/test/fork/ChainForkTest.sol @@ -33,7 +33,7 @@ abstract contract ChainForkTest is BaseForkTest { function test_brokerCanNotBeReinitialized() public { vm.expectRevert("contract already initialized"); - broker.initialize(new address[](0), address(reserve)); + broker.initialize(new address[](0), new address[](0)); } function test_sortedOraclesCanNotBeReinitialized() public { diff --git a/test/fork/helpers/TradingLimitHelpers.sol b/test/fork/helpers/TradingLimitHelpers.sol index 5f47adb..06ebb4d 100644 --- a/test/fork/helpers/TradingLimitHelpers.sol +++ b/test/fork/helpers/TradingLimitHelpers.sol @@ -14,7 +14,15 @@ library TradingLimitHelpers { using OracleHelpers for *; function isLimitConfigured(ExchangeForkTest ctx, bytes32 limitId) public view returns (bool) { - ITradingLimits.Config memory limitConfig = ctx.broker().tradingLimitsConfig(limitId); + ITradingLimits.Config memory limitConfig; + ( + limitConfig.timestep0, + limitConfig.timestep1, + limitConfig.limit0, + limitConfig.limit1, + limitConfig.limitGlobal, + limitConfig.flags + ) = ctx.broker().tradingLimitsConfig(limitId); return limitConfig.flags > uint8(0); } @@ -22,21 +30,59 @@ library TradingLimitHelpers { ExchangeForkTest ctx, bytes32 limitId ) public view returns (ITradingLimits.Config memory) { - return ctx.broker().tradingLimitsConfig(limitId); + ITradingLimits.Config memory limitConfig; + ( + limitConfig.timestep0, + limitConfig.timestep1, + limitConfig.limit0, + limitConfig.limit1, + limitConfig.limitGlobal, + limitConfig.flags + ) = ctx.broker().tradingLimitsConfig(limitId); + + return limitConfig; } function tradingLimitsState(ExchangeForkTest ctx, bytes32 limitId) public view returns (ITradingLimits.State memory) { - return ctx.broker().tradingLimitsState(limitId); + ITradingLimits.State memory limitState; + ( + limitState.lastUpdated0, + limitState.lastUpdated1, + limitState.netflow0, + limitState.netflow1, + limitState.netflowGlobal + ) = ctx.broker().tradingLimitsState(limitId); + return limitState; } function tradingLimitsConfig(ExchangeForkTest ctx, address asset) public view returns (ITradingLimits.Config memory) { + ITradingLimits.Config memory limitConfig; bytes32 assetBytes32 = bytes32(uint256(uint160(asset))); - return ctx.broker().tradingLimitsConfig(ctx.exchangeId() ^ assetBytes32); + bytes32 limitId = ctx.exchangeId() ^ assetBytes32; + + ( + limitConfig.timestep0, + limitConfig.timestep1, + limitConfig.limit0, + limitConfig.limit1, + limitConfig.limitGlobal, + limitConfig.flags + ) = ctx.broker().tradingLimitsConfig(limitId); + return limitConfig; } function tradingLimitsState(ExchangeForkTest ctx, address asset) public view returns (ITradingLimits.State memory) { + ITradingLimits.State memory limitState; bytes32 assetBytes32 = bytes32(uint256(uint160(asset))); - return ctx.broker().tradingLimitsState(ctx.exchangeId() ^ assetBytes32); + bytes32 limitId = ctx.exchangeId() ^ assetBytes32; + ( + limitState.lastUpdated0, + limitState.lastUpdated1, + limitState.netflow0, + limitState.netflow1, + limitState.netflowGlobal + ) = ctx.broker().tradingLimitsState(limitId); + return limitState; } function refreshedTradingLimitsState( diff --git a/test/integration/protocol/ProtocolTest.sol b/test/integration/protocol/ProtocolTest.sol index 0405855..668eda8 100644 --- a/test/integration/protocol/ProtocolTest.sol +++ b/test/integration/protocol/ProtocolTest.sol @@ -360,7 +360,10 @@ contract ProtocolTest is Test, WithRegistry { address[] memory exchangeProviders = new address[](1); exchangeProviders[0] = address(biPoolManager); - broker.initialize(exchangeProviders, address(reserve)); + address[] memory reserves = new address[](1); + reserves[0] = address(reserve); + + broker.initialize(exchangeProviders, reserves); registry.setAddressFor("Broker", address(broker)); reserve.addExchangeSpender(address(broker)); biPoolManager.setPricingModules(pricingModuleIdentifiers, pricingModules); diff --git a/test/unit/libraries/TradingLimits.t.sol b/test/unit/libraries/TradingLimits.t.sol index 136b2bc..878fdf2 100644 --- a/test/unit/libraries/TradingLimits.t.sol +++ b/test/unit/libraries/TradingLimits.t.sol @@ -6,7 +6,7 @@ pragma solidity ^0.8; import { Test } from "mento-std/Test.sol"; import { ITradingLimits } from "contracts/interfaces/ITradingLimits.sol"; -import { ITradingLimitsHarness } from "test/utils/harnesses/ITradingLimitsHarness.sol"; +import { TradingLimitsHarness } from "test/utils/harnesses/TradingLimitsHarness.sol"; // forge test --match-contract TradingLimits -vvv contract TradingLimitsTest is Test { @@ -15,7 +15,7 @@ contract TradingLimitsTest is Test { uint8 private constant LG = 4; // 0b100 ITradingLimits.State private state; - ITradingLimitsHarness private harness; + TradingLimitsHarness private harness; function configEmpty() internal pure returns (ITradingLimits.Config memory config) {} @@ -87,7 +87,7 @@ contract TradingLimitsTest is Test { } function setUp() public { - harness = ITradingLimitsHarness(deployCode("TradingLimitsHarness")); + harness = new TradingLimitsHarness(); } /* ==================== Config#validate ==================== */ @@ -286,11 +286,16 @@ contract TradingLimitsTest is Test { assertEq(state.netflowGlobal, 100); } - function test_update_withSubUnitAmounts_updatesAs1() public { + function test_update_withPositiveSubUnitAmounts_updatesAs1() public { state = harness.update(state, configLG(500000), 1e6, 18); assertEq(state.netflowGlobal, 1); } + function test_update_withNegativeSubUnitAmounts_updatesAsMinus1() public { + state = harness.update(state, configLG(500000), -1e6, 18); + assertEq(state.netflowGlobal, -1); + } + function test_update_withTooLargeAmount_reverts() public { vm.expectRevert(bytes("dFlow too large")); state = harness.update(state, configLG(500000), 3 * 10e32, 18); diff --git a/test/unit/swap/Broker.t.sol b/test/unit/swap/Broker.t.sol index 06ef2d8..fdbe65a 100644 --- a/test/unit/swap/Broker.t.sol +++ b/test/unit/swap/Broker.t.sol @@ -12,7 +12,7 @@ import { TestERC20 } from "test/utils/mocks/TestERC20.sol"; import { FixidityLib } from "celo/contracts/common/FixidityLib.sol"; import { IStableTokenV2 } from "contracts/interfaces/IStableTokenV2.sol"; -import { IBroker } from "contracts/interfaces/IBroker.sol"; +import { Broker } from "contracts/swap/Broker.sol"; import { ITradingLimits } from "contracts/interfaces/ITradingLimits.sol"; // forge test --match-contract Broker -vvv @@ -40,24 +40,27 @@ contract BrokerTest is Test { address randomAsset = makeAddr("randomAsset"); MockReserve reserve; + MockReserve reserve1; TestERC20 stableAsset; TestERC20 collateralAsset; - IBroker broker; + Broker broker; MockExchangeProvider exchangeProvider; address exchangeProvider1 = makeAddr("exchangeProvider1"); address exchangeProvider2 = makeAddr("exchangeProvider2"); address[] public exchangeProviders; + address[] public reserves; function setUp() public virtual { /* Dependencies and makeAddrs */ reserve = new MockReserve(); + reserve1 = new MockReserve(); collateralAsset = new TestERC20("Collateral", "CL"); stableAsset = new TestERC20("StableAsset", "SA0"); randomAsset = makeAddr("randomAsset"); - broker = IBroker(deployCode("Broker", abi.encode(true))); + broker = new Broker(true); exchangeProvider = new MockExchangeProvider(); reserve.addToken(address(stableAsset)); @@ -66,7 +69,12 @@ contract BrokerTest is Test { exchangeProviders.push(exchangeProvider1); exchangeProviders.push(exchangeProvider2); exchangeProviders.push((address(exchangeProvider))); - broker.initialize(exchangeProviders, address(reserve)); + reserves.push(address(reserve)); + reserves.push(address(reserve)); + reserves.push(address(reserve)); + + vm.prank(deployer); + broker.initialize(exchangeProviders, reserves); } } @@ -74,15 +82,16 @@ contract BrokerTest_initilizerAndSetters is BrokerTest { /* ---------- Initilizer ---------- */ function test_initilize_shouldSetOwner() public view { - assertEq(broker.owner(), address(this)); + assertEq(broker.owner(), deployer); } function test_initilize_shouldSetExchangeProviderAddresseses() public view { assertEq(broker.getExchangeProviders(), exchangeProviders); } - - function test_initilize_shouldSetReserve() public view { - assertEq(address(broker.reserve()), address(reserve)); + function test_initilize_shouldSetReserves() public { + assertEq(address(broker.exchangeReserve(exchangeProvider1)), address(reserve)); + assertEq(address(broker.exchangeReserve(exchangeProvider2)), address(reserve)); + assertEq(address(broker.exchangeReserve(address(exchangeProvider))), address(reserve)); } /* ---------- Setters ---------- */ @@ -90,43 +99,62 @@ contract BrokerTest_initilizerAndSetters is BrokerTest { function test_addExchangeProvider_whenSenderIsNotOwner_shouldRevert() public { vm.expectRevert("Ownable: caller is not the owner"); vm.prank(notDeployer); - broker.addExchangeProvider(address(0)); + broker.addExchangeProvider(address(0), address(0)); } - function test_addExchangeProvider_whenAddressIsZero_shouldRevert() public { + function test_addExchangeProvider_whenExchangeProviderAddressIsZero_shouldRevert() public { vm.expectRevert("ExchangeProvider address can't be 0"); - broker.addExchangeProvider(address(0)); + vm.prank(deployer); + broker.addExchangeProvider(address(0), address(reserve)); + } + + function test_addExchangeProvider_whenReserveAddressIsZero_shouldRevert() public { + changePrank(deployer); + vm.expectRevert("Reserve address can't be 0"); + broker.addExchangeProvider(makeAddr("newExchangeProvider"), address(0)); } function test_addExchangeProvider_whenSenderIsOwner_shouldUpdateAndEmit() public { address newExchangeProvider = makeAddr("newExchangeProvider"); - vm.expectEmit(true, false, false, false); + + vm.expectEmit(true, true, true, true); emit ExchangeProviderAdded(newExchangeProvider); - broker.addExchangeProvider(newExchangeProvider); + vm.expectEmit(true, true, true, true); + emit ReserveSet(newExchangeProvider, address(reserve1)); + + vm.prank(deployer); + broker.addExchangeProvider(newExchangeProvider, address(reserve1)); + address[] memory updatedExchangeProviders = broker.getExchangeProviders(); assertEq(updatedExchangeProviders[updatedExchangeProviders.length - 1], newExchangeProvider); assertEq(broker.isExchangeProvider(newExchangeProvider), true); + assertEq(broker.exchangeReserve(newExchangeProvider), address(reserve1)); } function test_addExchangeProvider_whenAlreadyAdded_shouldRevert() public { vm.expectRevert("ExchangeProvider already exists in the list"); - broker.addExchangeProvider(address(exchangeProvider)); + vm.prank(deployer); + broker.addExchangeProvider(address(exchangeProvider), address(reserve1)); } function test_removeExchangeProvider_whenSenderIsOwner_shouldUpdateAndEmit() public { vm.expectEmit(true, true, true, true); emit ExchangeProviderRemoved(exchangeProvider1); + vm.prank(deployer); broker.removeExchangeProvider(exchangeProvider1, 0); assert(broker.getExchangeProviders()[0] != exchangeProvider1); + assertEq(broker.exchangeReserve(exchangeProvider1), address(0)); } function test_removeExchangeProvider_whenAddressDoesNotExist_shouldRevert() public { vm.expectRevert("index doesn't match provider"); + vm.prank(deployer); broker.removeExchangeProvider(notDeployer, 1); } function test_removeExchangeProvider_whenIndexOutOfRange_shouldRevert() public { vm.expectRevert("index doesn't match provider"); + vm.prank(deployer); broker.removeExchangeProvider(exchangeProvider1, 1); } @@ -136,24 +164,48 @@ contract BrokerTest_initilizerAndSetters is BrokerTest { broker.removeExchangeProvider(exchangeProvider1, 0); } - function test_setReserve_whenSenderIsNotOwner_shouldRevert() public { + function test_setReserves_whenSenderIsNotOwner_shouldRevert() public { vm.prank(notDeployer); vm.expectRevert("Ownable: caller is not the owner"); - broker.setReserve(address(0)); + broker.setReserves(new address[](0), new address[](0)); } - function test_setReserve_whenAddressIsZero_shouldRevert() public { - vm.expectRevert("Reserve address must be set"); - broker.setReserve(address(0)); + function test_setReserves_whenExchangeProviderIsNotAdded_shouldRevert() public { + address[] memory exchangeProviders = new address[](1); + exchangeProviders[0] = makeAddr("newExchangeProvider"); + address[] memory reserves = new address[](1); + reserves[0] = makeAddr("newReserve"); + changePrank(deployer); + vm.expectRevert("ExchangeProvider does not exist"); + broker.setReserves(exchangeProviders, reserves); + } + + function test_setReserves_whenReserveAddressIsZero_shouldRevert() public { + address[] memory exchangeProviders = new address[](1); + exchangeProviders[0] = exchangeProvider1; + address[] memory reserves = new address[](1); + reserves[0] = address(0); + changePrank(deployer); + vm.expectRevert("Reserve address can't be 0"); + broker.setReserves(exchangeProviders, reserves); } - function test_setReserve_whenSenderIsOwner_shouldUpdateAndEmit() public { - address newReserve = makeAddr("newReserve"); - vm.expectEmit(true, false, false, false); - emit ReserveSet(newReserve, address(reserve)); + function test_setReserves_whenSenderIsOwner_shouldUpdateAndEmit() public { + address[] memory exchangeProviders = new address[](2); + exchangeProviders[0] = exchangeProvider1; + exchangeProviders[1] = exchangeProvider2; - broker.setReserve(newReserve); - assertEq(address(broker.reserve()), newReserve); + address[] memory reserves = new address[](2); + reserves[0] = makeAddr("newReserve"); + reserves[1] = makeAddr("newReserve2"); + changePrank(deployer); + vm.expectEmit(true, true, true, true); + emit ReserveSet(exchangeProvider1, reserves[0]); + vm.expectEmit(true, true, true, true); + emit ReserveSet(exchangeProvider2, reserves[1]); + broker.setReserves(exchangeProviders, reserves); + assertEq(address(broker.exchangeReserve(address(exchangeProvider1))), reserves[0]); + assertEq(address(broker.exchangeReserve(address(exchangeProvider2))), reserves[1]); } } @@ -219,12 +271,6 @@ contract BrokerTest_getAmounts is BrokerTest { contract BrokerTest_BurnStableTokens is BrokerTest { uint256 burnAmount = 1; - function test_burnStableTokens_whenTokenIsNotReserveStable_shouldRevert() public { - vm.prank(notDeployer); - vm.expectRevert("Token must be a reserve stable asset"); - broker.burnStableTokens(randomAsset, 2); - } - function test_burnStableTokens_whenTokenIsAReserveStable_shouldBurnAndEmit() public { stableAsset.mint(notDeployer, 2); vm.prank(notDeployer); @@ -488,6 +534,7 @@ contract BrokerTest_swap is BrokerTest { vm.expectEmit(true, true, true, true); emit TradingLimitConfigured(exchangeId, address(stableAsset), config); + vm.prank(deployer); broker.configureTradingLimit(exchangeId, address(stableAsset), config); vm.prank(trader); @@ -504,6 +551,7 @@ contract BrokerTest_swap is BrokerTest { vm.expectEmit(true, true, true, true); emit TradingLimitConfigured(exchangeId, address(stableAsset), config); + vm.prank(deployer); broker.configureTradingLimit(exchangeId, address(stableAsset), config); vm.expectRevert(bytes("L0 Exceeded")); diff --git a/test/utils/harnesses/TradingLimitsHarness.sol b/test/utils/harnesses/TradingLimitsHarness.sol index 723ac7a..047a0f2 100644 --- a/test/utils/harnesses/TradingLimitsHarness.sol +++ b/test/utils/harnesses/TradingLimitsHarness.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later -pragma solidity ^0.5.13; +pragma solidity 0.8.18; pragma experimental ABIEncoderV2; import { TradingLimits } from "contracts/libraries/TradingLimits.sol"; @@ -10,18 +10,18 @@ contract TradingLimitsHarness is ITradingLimitsHarness { using TradingLimits for ITradingLimits.State; using TradingLimits for ITradingLimits.Config; - function validate(ITradingLimits.Config memory config) public view { + function validate(ITradingLimits.Config memory config) public pure { return config.validate(); } - function verify(ITradingLimits.State memory state, ITradingLimits.Config memory config) public view { + function verify(ITradingLimits.State memory state, ITradingLimits.Config memory config) public pure { return state.verify(config); } function reset( ITradingLimits.State memory state, ITradingLimits.Config memory config - ) public view returns (ITradingLimits.State memory) { + ) public pure returns (ITradingLimits.State memory) { return state.reset(config); }