From 93ba4f69513295bab85fb1b8047b583d0b2b4a1c Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 11:51:43 +0200 Subject: [PATCH 1/7] Remove `StakeTokenizer` (#70) --- contracts/erc20/base/ERC20.sol | 235 ------------------------- contracts/erc20/base/ERC20Burnable.sol | 27 --- contracts/erc20/base/ERC20Detailed.sol | 58 ------ contracts/erc20/base/ERC20Mintable.sol | 48 ----- contracts/erc20/base/IERC20.sol | 77 -------- contracts/erc20/base/MinterRole.sol | 38 ---- contracts/erc20/base/Roles.sol | 37 ---- contracts/sfc/SFC.sol | 8 - contracts/sfc/SFCI.sol | 8 - contracts/sfc/SFCLib.sol | 43 ----- contracts/sfc/SFCState.sol | 4 - contracts/sfc/StakeTokenizer.sol | 61 ------- contracts/test/UnitTestSFC.sol | 4 - 13 files changed, 648 deletions(-) delete mode 100644 contracts/erc20/base/ERC20.sol delete mode 100644 contracts/erc20/base/ERC20Burnable.sol delete mode 100644 contracts/erc20/base/ERC20Detailed.sol delete mode 100644 contracts/erc20/base/ERC20Mintable.sol delete mode 100644 contracts/erc20/base/IERC20.sol delete mode 100644 contracts/erc20/base/MinterRole.sol delete mode 100644 contracts/erc20/base/Roles.sol delete mode 100644 contracts/sfc/StakeTokenizer.sol diff --git a/contracts/erc20/base/ERC20.sol b/contracts/erc20/base/ERC20.sol deleted file mode 100644 index 71dfb52..0000000 --- a/contracts/erc20/base/ERC20.sol +++ /dev/null @@ -1,235 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "./IERC20.sol"; -import "../../common/Initializable.sol"; - -/** - * @dev Implementation of the {IERC20} interface. - * - * This implementation is agnostic to the way tokens are created. This means - * that a supply mechanism has to be added in a derived contract using {_mint}. - * For a generic mechanism see {ERC20Mintable}. - * - * TIP: For a detailed writeup see our guide - * https://forum.zeppelin.solutions/t/how-to-implement-erc20-supply-mechanisms/226[How - * to implement supply mechanisms]. - * - * We have followed general OpenZeppelin guidelines: functions revert instead - * of returning `false` on failure. This behavior is nonetheless conventional - * and does not conflict with the expectations of ERC20 applications. - * - * Additionally, an {Approval} event is emitted on calls to {transferFrom}. - * This allows applications to reconstruct the allowance for all accounts just - * by listening to said events. Other implementations of the EIP may not emit - * these events, as it isn't required by the specification. - * - * Finally, the non-standard {decreaseAllowance} and {increaseAllowance} - * functions have been added to mitigate the well-known issues around setting - * allowances. See {IERC20-approve}. - */ -contract ERC20 is Initializable, IERC20 { - mapping(address => uint256) private _balances; - - mapping(address => mapping(address => uint256)) private _allowances; - - uint256 private _totalSupply; - - /** - * @dev See {IERC20-totalSupply}. - */ - function totalSupply() public view returns (uint256) { - return _totalSupply; - } - - /** - * @dev See {IERC20-balanceOf}. - */ - function balanceOf(address account) public view returns (uint256) { - return _balances[account]; - } - - /** - * @dev See {IERC20-transfer}. - * - * Requirements: - * - * - `recipient` cannot be the zero address. - * - the caller must have a balance of at least `amount`. - */ - function transfer(address recipient, uint256 amount) public returns (bool) { - _transfer(msg.sender, recipient, amount); - return true; - } - - /** - * @dev See {IERC20-allowance}. - */ - function allowance(address owner, address spender) public view returns (uint256) { - return _allowances[owner][spender]; - } - - /** - * @dev See {IERC20-approve}. - * - * Requirements: - * - * - `spender` cannot be the zero address. - */ - function approve(address spender, uint256 amount) public returns (bool) { - _approve(msg.sender, spender, amount); - return true; - } - - /** - * @dev See {IERC20-transferFrom}. - * - * Emits an {Approval} event indicating the updated allowance. This is not - * required by the EIP. See the note at the beginning of {ERC20}; - * - * Requirements: - * - `sender` and `recipient` cannot be the zero address. - * - `sender` must have a balance of at least `amount`. - * - the caller must have allowance for `sender`'s tokens of at least - * `amount`. - */ - function transferFrom(address sender, address recipient, uint256 amount) public returns (bool) { - _transfer(sender, recipient, amount); - require(amount <= _allowances[sender][msg.sender], "ERC20: transfer amount exceeds allowance"); - _approve(sender, msg.sender, _allowances[sender][msg.sender] - amount); - return true; - } - - /** - * @dev Atomically increases the allowance granted to `spender` by the caller. - * - * This is an alternative to {approve} that can be used as a mitigation for - * problems described in {IERC20-approve}. - * - * Emits an {Approval} event indicating the updated allowance. - * - * Requirements: - * - * - `spender` cannot be the zero address. - */ - function increaseAllowance(address spender, uint256 addedValue) public returns (bool) { - _approve(msg.sender, spender, _allowances[msg.sender][spender] + addedValue); - return true; - } - - /** - * @dev Atomically decreases the allowance granted to `spender` by the caller. - * - * This is an alternative to {approve} that can be used as a mitigation for - * problems described in {IERC20-approve}. - * - * Emits an {Approval} event indicating the updated allowance. - * - * Requirements: - * - * - `spender` cannot be the zero address. - * - `spender` must have allowance for the caller of at least - * `subtractedValue`. - */ - function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) { - require(subtractedValue <= _allowances[msg.sender][spender], "ERC20: decreased allowance below zero"); - _approve(msg.sender, spender, _allowances[msg.sender][spender] - subtractedValue); - return true; - } - - /** - * @dev Moves tokens `amount` from `sender` to `recipient`. - * - * This is internal function is equivalent to {transfer}, and can be used to - * e.g. implement automatic token fees, slashing mechanisms, etc. - * - * Emits a {Transfer} event. - * - * Requirements: - * - * - `sender` cannot be the zero address. - * - `recipient` cannot be the zero address. - * - `sender` must have a balance of at least `amount`. - */ - function _transfer(address sender, address recipient, uint256 amount) internal { - require(sender != address(0), "ERC20: transfer from the zero address"); - require(recipient != address(0), "ERC20: transfer to the zero address"); - - require(_balances[sender] >= amount, "ERC20: transfer amount exceeds balance"); - _balances[sender] = _balances[sender] - amount; - _balances[recipient] = _balances[recipient] + amount; - emit Transfer(sender, recipient, amount); - } - - /** @dev Creates `amount` tokens and assigns them to `account`, increasing - * the total supply. - * - * Emits a {Transfer} event with `from` set to the zero address. - * - * Requirements - * - * - `to` cannot be the zero address. - */ - function _mint(address account, uint256 amount) internal { - require(account != address(0), "ERC20: mint to the zero address"); - - _totalSupply = _totalSupply + amount; - _balances[account] = _balances[account] + amount; - emit Transfer(address(0), account, amount); - } - - /** - * @dev Destroys `amount` tokens from `account`, reducing the - * total supply. - * - * Emits a {Transfer} event with `to` set to the zero address. - * - * Requirements - * - * - `account` cannot be the zero address. - * - `account` must have at least `amount` tokens. - */ - function _burn(address account, uint256 amount) internal { - require(account != address(0), "ERC20: burn from the zero address"); - - require(_balances[account] >= amount, "ERC20: burn amount exceeds balance"); - _balances[account] = _balances[account] - amount; - _totalSupply = _totalSupply - amount; - emit Transfer(account, address(0), amount); - } - - /** - * @dev Sets `amount` as the allowance of `spender` over the `owner`s tokens. - * - * This is internal function is equivalent to `approve`, and can be used to - * e.g. set automatic allowances for certain subsystems, etc. - * - * Emits an {Approval} event. - * - * Requirements: - * - * - `owner` cannot be the zero address. - * - `spender` cannot be the zero address. - */ - function _approve(address owner, address spender, uint256 amount) internal { - require(owner != address(0), "ERC20: approve from the zero address"); - require(spender != address(0), "ERC20: approve to the zero address"); - - _allowances[owner][spender] = amount; - emit Approval(owner, spender, amount); - } - - /** - * @dev Destroys `amount` tokens from `account`.`amount` is then deducted - * from the caller's allowance. - * - * See {_burn} and {_approve}. - */ - function _burnFrom(address account, uint256 amount) internal { - _burn(account, amount); - require(amount <= _allowances[account][msg.sender], "ERC20: burn amount exceeds allowance"); - _approve(account, msg.sender, _allowances[account][msg.sender] - amount); - } - - uint256[50] private ______gap; -} diff --git a/contracts/erc20/base/ERC20Burnable.sol b/contracts/erc20/base/ERC20Burnable.sol deleted file mode 100644 index 9b5aa0f..0000000 --- a/contracts/erc20/base/ERC20Burnable.sol +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "./ERC20.sol"; - -/** - * @title Burnable Token - * @dev Token that can be irreversibly burned (destroyed). - */ -contract ERC20Burnable is ERC20 { - /** - * @dev Burns a specific amount of tokens. - * @param value The amount of token to be burned. - */ - function burn(uint256 value) public { - _burn(msg.sender, value); - } - - /** - * @dev Burns a specific amount of tokens from the target address and decrements allowance - * @param from address The address which you want to send tokens from - * @param value uint256 The amount of token to be burned - */ - function burnFrom(address from, uint256 value) public { - _burnFrom(from, value); - } -} diff --git a/contracts/erc20/base/ERC20Detailed.sol b/contracts/erc20/base/ERC20Detailed.sol deleted file mode 100644 index 1380dcf..0000000 --- a/contracts/erc20/base/ERC20Detailed.sol +++ /dev/null @@ -1,58 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "../../common/Initializable.sol"; -import {ERC20} from "./ERC20.sol"; - -/** - * @dev Optional functions from the ERC20 standard. - */ -contract ERC20Detailed is ERC20 { - string private _name; - string private _symbol; - uint8 private _decimals; - - /** - * @dev Sets the values for `name`, `symbol`, and `decimals`. All three of - * these values are immutable: they can only be set once during - * construction. - */ - function initialize(string memory tokenName, string memory tokenSymbol, uint8 tokenDecimals) internal initializer { - _name = tokenName; - _symbol = tokenSymbol; - _decimals = tokenDecimals; - } - - /** - * @dev Returns the name of the token. - */ - function name() public view returns (string memory) { - return _name; - } - - /** - * @dev Returns the symbol of the token, usually a shorter version of the - * name. - */ - function symbol() public view returns (string memory) { - return _symbol; - } - - /** - * @dev Returns the number of decimals used to get its user representation. - * For example, if `decimals` equals `2`, a balance of `505` tokens should - * be displayed to a user as `5,05` (`505 / 10 ** 2`). - * - * Tokens usually opt for a value of 18, imitating the relationship between - * Ether and Wei. - * - * NOTE: This information is only used for _display_ purposes: it in - * no way affects any of the arithmetic of the contract, including - * {IERC20-balanceOf} and {IERC20-transfer}. - */ - function decimals() public view returns (uint8) { - return _decimals; - } - - uint256[50] private ______gap; -} diff --git a/contracts/erc20/base/ERC20Mintable.sol b/contracts/erc20/base/ERC20Mintable.sol deleted file mode 100644 index 63981be..0000000 --- a/contracts/erc20/base/ERC20Mintable.sol +++ /dev/null @@ -1,48 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "./ERC20.sol"; -import "./MinterRole.sol"; - -/** - * @title ERC20Mintable - * @dev ERC20 minting logic - */ -contract ERC20Mintable is ERC20, MinterRole { - event MintingFinished(); - - bool private _mintingFinished = false; - - modifier onlyBeforeMintingFinished() { - require(!_mintingFinished); - _; - } - - /** - * @return true if the minting is finished. - */ - function mintingFinished() public view returns (bool) { - return _mintingFinished; - } - - /** - * @dev Function to mint tokens - * @param to The address that will receive the minted tokens. - * @param amount The amount of tokens to mint. - * @return A boolean that indicates if the operation was successful. - */ - function mint(address to, uint256 amount) public onlyMinter onlyBeforeMintingFinished returns (bool) { - _mint(to, amount); - return true; - } - - /** - * @dev Function to stop minting new tokens. - * @return True if the operation was successful. - */ - function finishMinting() public onlyMinter onlyBeforeMintingFinished returns (bool) { - _mintingFinished = true; - emit MintingFinished(); - return true; - } -} diff --git a/contracts/erc20/base/IERC20.sol b/contracts/erc20/base/IERC20.sol deleted file mode 100644 index 5fbe198..0000000 --- a/contracts/erc20/base/IERC20.sol +++ /dev/null @@ -1,77 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED - -pragma solidity ^0.8.9; - -/** - * @dev Interface of the ERC20 standard as defined in the EIP. - */ -interface IERC20 { - /** - * @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); - - /** - * @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/erc20/base/MinterRole.sol b/contracts/erc20/base/MinterRole.sol deleted file mode 100644 index 627a786..0000000 --- a/contracts/erc20/base/MinterRole.sol +++ /dev/null @@ -1,38 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "./Roles.sol"; - -contract MinterRole { - using Roles for Roles.Role; - - event MinterAdded(address indexed account); - event MinterRemoved(address indexed account); - - Roles.Role private minters; - - modifier onlyMinter() { - require(isMinter(msg.sender)); - _; - } - - function isMinter(address account) public view returns (bool) { - return minters.has(account); - } - - function renounceMinter() public { - minters.remove(msg.sender); - } - - function _removeMinter(address account) internal { - minters.remove(account); - emit MinterRemoved(account); - } - - function _addMinter(address account) internal { - minters.add(account); - emit MinterAdded(account); - } - - uint256[50] private ______gap; -} diff --git a/contracts/erc20/base/Roles.sol b/contracts/erc20/base/Roles.sol deleted file mode 100644 index 77bea09..0000000 --- a/contracts/erc20/base/Roles.sol +++ /dev/null @@ -1,37 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -/** - * @title Roles - * @dev Library for managing addresses assigned to a Role. - */ -library Roles { - struct Role { - mapping(address => bool) bearer; - } - - /** - * @dev give an account access to this role - */ - function add(Role storage role, address account) internal { - require(account != address(0)); - role.bearer[account] = true; - } - - /** - * @dev remove an account's access to this role - */ - function remove(Role storage role, address account) internal { - require(account != address(0)); - role.bearer[account] = false; - } - - /** - * @dev check if an account has this role - * @return bool - */ - function has(Role storage role, address account) internal view returns (bool) { - require(account != address(0)); - return role.bearer[account]; - } -} diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index 42de7f4..eff0b6c 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -102,10 +102,6 @@ contract SFC is SFCBase, Version { getEpochSnapshot[sealedEpoch].endTime = _now(); } - function updateStakeTokenizerAddress(address addr) external onlyOwner { - stakeTokenizerAddress = addr; - } - function updateLibAddress(address v) external onlyOwner { libAddress = v; } @@ -126,10 +122,6 @@ contract SFC is SFCBase, Version { voteBookAddress = v; } - function updateSFTMFinalizer(address v) external onlyOwner { - sftmFinalizer = v; - } - function migrateValidatorPubkeyUniquenessFlag(uint256 start, uint256 end) external { for (uint256 vid = start; vid < end; vid++) { bytes memory pubkey = getValidatorPubkey[vid]; diff --git a/contracts/sfc/SFCI.sol b/contracts/sfc/SFCI.sol index 9ca168b..40d90f2 100644 --- a/contracts/sfc/SFCI.sol +++ b/contracts/sfc/SFCI.sol @@ -101,8 +101,6 @@ interface SFCI { function slashingRefundRatio(uint256) external view returns (uint256); - function stakeTokenizerAddress() external view returns (address); - function stashedRewardsUntilEpoch(address, uint256) external view returns (uint256); function totalActiveStake() external view returns (uint256); @@ -171,8 +169,6 @@ interface SFCI { function updateSlashingRefundRatio(uint256 validatorID, uint256 refundRatio) external; - function updateStakeTokenizerAddress(address addr) external; - function updateTreasuryAddress(address v) external; function burnFTM(uint256 amount) external; @@ -233,10 +229,6 @@ interface SFCI { function voteBookAddress() external view returns (address); - function liquidateSFTM(address delegator, uint256 toValidatorID, uint256 amount) external; - - function updateSFTMFinalizer(address v) external; - function updateValidatorPubkey(bytes calldata pubkey) external; function migrateValidatorPubkeyUniquenessFlag(uint256 start, uint256 end) external; diff --git a/contracts/sfc/SFCLib.sol b/contracts/sfc/SFCLib.sol index 4ee8f77..231d767 100644 --- a/contracts/sfc/SFCLib.sol +++ b/contracts/sfc/SFCLib.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.9; import "../common/Decimal.sol"; import "./GasPriceConstants.sol"; import "./SFCBase.sol"; -import "./StakeTokenizer.sol"; import "./NodeDriver.sol"; contract SFCLib is SFCBase { @@ -229,7 +228,6 @@ contract SFCLib is SFCBase { require(amount > 0, "zero amount"); require(amount <= getUnlockedStake(delegator, toValidatorID), "not enough unlocked stake"); - require(_checkAllowedToWithdraw(delegator, toValidatorID), "outstanding sFTM balance"); require(getWithdrawalRequest[delegator][toValidatorID][wrID].amount == 0, "wrID already exists"); @@ -244,37 +242,6 @@ contract SFCLib is SFCBase { emit Undelegated(delegator, toValidatorID, wrID, amount); } - // liquidateSFTM is used for finalization of last fMint positions with outstanding sFTM balances - // it allows to undelegate without the unboding period, and also to unlock stake without a penalty. - // Such a simplification, which might be dangerous generally, is okay here because there's only a small amount - // of leftover sFTM - function liquidateSFTM(address delegator, uint256 toValidatorID, uint256 amount) external { - require(msg.sender == sftmFinalizer, "not sFTM finalizer"); - _stashRewards(delegator, toValidatorID); - - require(amount > 0, "zero amount"); - StakeTokenizer(stakeTokenizerAddress).redeemSFTMFor(msg.sender, delegator, toValidatorID, amount); - require(amount <= getStake[delegator][toValidatorID], "not enough stake"); - uint256 unlockedStake = getUnlockedStake(delegator, toValidatorID); - if (amount > unlockedStake) { - LockedDelegation storage ld = getLockupInfo[delegator][toValidatorID]; - ld.lockedStake = ld.lockedStake - amount - unlockedStake; - emit UnlockedStake(delegator, toValidatorID, amount - unlockedStake, 0); - } - - _rawUndelegate(delegator, toValidatorID, amount, false, true, false); - - _syncValidator(toValidatorID, false); - - emit Undelegated(delegator, toValidatorID, 0xffffffffff, amount); - - // It's important that we transfer after erasing (protection against Re-Entrancy) - (bool sent, ) = msg.sender.call{value: amount}(""); - require(sent, "Failed to send FTM"); - - emit Withdrawn(delegator, toValidatorID, 0xffffffffff, amount); - } - function isSlashed(uint256 validatorID) public view returns (bool) { return getValidator[validatorID].status & CHEATER_MASK != 0; } @@ -298,7 +265,6 @@ contract SFCLib is SFCBase { function _withdraw(address delegator, uint256 toValidatorID, uint256 wrID, address payable receiver) private { WithdrawalRequest memory request = getWithdrawalRequest[delegator][toValidatorID][wrID]; require(request.epoch != 0, "request doesn't exist"); - require(_checkAllowedToWithdraw(delegator, toValidatorID), "outstanding sFTM balance"); uint256 requestTime = request.time; uint256 requestEpoch = request.epoch; @@ -455,7 +421,6 @@ contract SFCLib is SFCBase { } function _claimRewards(address delegator, uint256 toValidatorID) internal returns (Rewards memory rewards) { - require(_checkAllowedToWithdraw(delegator, toValidatorID), "outstanding sFTM balance"); _stashRewards(delegator, toValidatorID); rewards = _rewardsStash[delegator][toValidatorID]; uint256 totalReward = rewards.unlockedReward + rewards.lockupBaseReward + rewards.lockupExtraReward; @@ -522,13 +487,6 @@ contract SFCLib is SFCBase { epochEndTime(epoch) <= getLockupInfo[delegator][toValidatorID].endTime; } - function _checkAllowedToWithdraw(address delegator, uint256 toValidatorID) internal view returns (bool) { - if (stakeTokenizerAddress == address(0)) { - return true; - } - return StakeTokenizer(stakeTokenizerAddress).allowedToWithdrawStake(delegator, toValidatorID); - } - function getUnlockedStake(address delegator, uint256 toValidatorID) public view returns (uint256) { if (!isLockedUp(delegator, toValidatorID)) { return getStake[delegator][toValidatorID]; @@ -653,7 +611,6 @@ contract SFCLib is SFCBase { require(amount > 0, "zero amount"); require(isLockedUp(delegator, toValidatorID), "not locked up"); require(amount <= ld.lockedStake, "not enough locked stake"); - require(_checkAllowedToWithdraw(delegator, toValidatorID), "outstanding sFTM balance"); require(!_redirected(delegator), "redirected"); _stashRewards(delegator, toValidatorID); diff --git a/contracts/sfc/SFCState.sol b/contracts/sfc/SFCState.sol index f270769..d5bf112 100644 --- a/contracts/sfc/SFCState.sol +++ b/contracts/sfc/SFCState.sol @@ -88,8 +88,6 @@ contract SFCState is Initializable, Ownable { mapping(uint256 => uint256) public slashingRefundRatio; // validator ID -> (slashing refund ratio) - address public stakeTokenizerAddress; - uint256 private erased3; uint256 private erased4; uint256 public minGasPrice; @@ -102,8 +100,6 @@ contract SFCState is Initializable, Ownable { address public voteBookAddress; - address internal sftmFinalizer; - struct Penalty { uint256 amount; uint256 end; diff --git a/contracts/sfc/StakeTokenizer.sol b/contracts/sfc/StakeTokenizer.sol deleted file mode 100644 index 0b831b4..0000000 --- a/contracts/sfc/StakeTokenizer.sol +++ /dev/null @@ -1,61 +0,0 @@ -// SPDX-License-Identifier: UNLICENSED -pragma solidity ^0.8.9; - -import "./SFC.sol"; -import "../erc20/base/ERC20Burnable.sol"; -import "../erc20/base/ERC20Mintable.sol"; -import "../common/Initializable.sol"; - -contract Spacer { - address private _owner; -} - -contract StakeTokenizer is Spacer, Initializable { - SFC internal sfc; - - mapping(address => mapping(uint256 => uint256)) public outstandingSFTM; - - address public sFTMTokenAddress; - - function initialize(address payable _sfc, address _sFTMTokenAddress) public initializer { - sfc = SFC(_sfc); - sFTMTokenAddress = _sFTMTokenAddress; - } - - function mintSFTM(uint256 toValidatorID) external { - revert("sFTM minting is disabled"); - // address delegator = msg.sender; - // uint256 lockedStake = sfc.getLockedStake(delegator, toValidatorID); - // require(lockedStake > 0, "delegation isn't locked up"); - // require(lockedStake > outstandingSFTM[delegator][toValidatorID], "sFTM is already minted"); - // - // uint256 diff = lockedStake - outstandingSFTM[delegator][toValidatorID]; - // outstandingSFTM[delegator][toValidatorID] = lockedStake; - // - // // It's important that we mint after updating outstandingSFTM (protection against Re-Entrancy) - // require(ERC20Mintable(sFTMTokenAddress).mint(delegator, diff), "failed to mint sFTM"); - } - - function redeemSFTM(uint256 validatorID, uint256 amount) external { - require(outstandingSFTM[msg.sender][validatorID] >= amount, "low outstanding sFTM balance"); - require(IERC20(sFTMTokenAddress).allowance(msg.sender, address(this)) >= amount, "insufficient allowance"); - outstandingSFTM[msg.sender][validatorID] -= amount; - - // It's important that we burn after updating outstandingSFTM (protection against Re-Entrancy) - ERC20Burnable(sFTMTokenAddress).burnFrom(msg.sender, amount); - } - - function redeemSFTMFor(address payer, address delegator, uint256 validatorID, uint256 amount) external { - require(msg.sender == address(sfc), "not SFC"); - require(outstandingSFTM[delegator][validatorID] >= amount, "low outstanding sFTM balance"); - require(IERC20(sFTMTokenAddress).allowance(payer, address(this)) >= amount, "insufficient allowance"); - outstandingSFTM[delegator][validatorID] -= amount; - - // It's important that we burn after updating outstandingSFTM (protection against Re-Entrancy) - ERC20Burnable(sFTMTokenAddress).burnFrom(payer, amount); - } - - function allowedToWithdrawStake(address sender, uint256 validatorID) public view returns (bool) { - return outstandingSFTM[sender][validatorID] == 0; - } -} diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol index a64d325..88374bb 100644 --- a/contracts/test/UnitTestSFC.sol +++ b/contracts/test/UnitTestSFC.sol @@ -177,8 +177,6 @@ interface SFCUnitTestI { function slashingRefundRatio(uint256) external view returns (uint256); - function stakeTokenizerAddress() external view returns (address); - function stashedRewardsUntilEpoch(address, uint256) external view returns (uint256); function targetGasPowerPerSecond() external view returns (uint256); @@ -251,8 +249,6 @@ interface SFCUnitTestI { function updateSlashingRefundRatio(uint256 validatorID, uint256 refundRatio) external; - function updateStakeTokenizerAddress(address addr) external; - function updateTreasuryAddress(address v) external; function burnFTM(uint256 amount) external; From c0e1d9d962cc5931fd79dbcceb07f51d4b03ebce Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 11:54:49 +0200 Subject: [PATCH 2/7] Remove unused variables (#71) --- contracts/sfc/ConstantsManager.sol | 9 --------- contracts/sfc/NodeDriver.sol | 1 - contracts/sfc/SFCState.sol | 6 ------ 3 files changed, 16 deletions(-) diff --git a/contracts/sfc/ConstantsManager.sol b/contracts/sfc/ConstantsManager.sol index 866a3f2..7a7b8d3 100644 --- a/contracts/sfc/ConstantsManager.sol +++ b/contracts/sfc/ConstantsManager.sol @@ -32,19 +32,10 @@ contract ConstantsManager is Ownable { uint256 public targetGasPowerPerSecond; uint256 public gasPriceBalancingCounterweight; - address private secondaryOwner_erased; - - // event SecondaryOwnershipTransferred(address indexed previousOwner, address indexed newOwner); - function initialize() external initializer { Ownable.initialize(msg.sender); } - // function setSecondaryOwner(address v) onlyOwner external { - // emit SecondaryOwnershipTransferred(secondaryOwner, v); - // secondaryOwner = v; - // } - function updateMinSelfStake(uint256 v) external virtual onlyOwner { require(v >= 100000 * 1e18, "too small value"); require(v <= 10000000 * 1e18, "too large value"); diff --git a/contracts/sfc/NodeDriver.sol b/contracts/sfc/NodeDriver.sol index 6e91706..5bfe390 100644 --- a/contracts/sfc/NodeDriver.sol +++ b/contracts/sfc/NodeDriver.sol @@ -228,7 +228,6 @@ contract NodeDriverAuth is Initializable, Ownable { } contract NodeDriver is Initializable { - uint256 private erased0; NodeDriverAuth internal backend; EVMWriter internal evmWriter; diff --git a/contracts/sfc/SFCState.sol b/contracts/sfc/SFCState.sol index d5bf112..ec74fdd 100644 --- a/contracts/sfc/SFCState.sol +++ b/contracts/sfc/SFCState.sol @@ -79,17 +79,11 @@ contract SFCState is Initializable, Ownable { uint256 totalSupply; } - uint256 private erased0; uint256 public totalSupply; mapping(uint256 => EpochSnapshot) public getEpochSnapshot; - uint256 private erased1; - uint256 private erased2; - mapping(uint256 => uint256) public slashingRefundRatio; // validator ID -> (slashing refund ratio) - uint256 private erased3; - uint256 private erased4; uint256 public minGasPrice; address public treasuryAddress; From 582037b9c82ffe8875859ef69717c2c4ccd5a341 Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 12:00:38 +0200 Subject: [PATCH 3/7] Add epoch snapshot `endBlock` attribute (#72) --- contracts/sfc/SFC.sol | 5 +++++ contracts/sfc/SFCI.sol | 3 +++ contracts/sfc/SFCState.sol | 1 + contracts/test/UnitTestSFC.sol | 3 +++ test/SFC.ts | 10 ++++++++++ 5 files changed, 22 insertions(+) diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index eff0b6c..0f7e31d 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -75,6 +75,10 @@ contract SFC is SFCBase, Version { return getEpochSnapshot[epoch].offlineBlocks[validatorID]; } + function getEpochEndBlock(uint256 epoch) public view returns (uint256) { + return getEpochSnapshot[epoch].endBlock; + } + function rewardsStash(address delegator, uint256 validatorID) public view returns (uint256) { Rewards memory stash = _rewardsStash[delegator][validatorID]; return stash.lockupBaseReward + stash.lockupExtraReward + stash.unlockedReward; @@ -361,6 +365,7 @@ contract SFC is SFCBase, Version { currentSealedEpoch = currentEpoch(); snapshot.endTime = _now(); + snapshot.endBlock = block.number; snapshot.baseRewardPerSecond = c.baseRewardPerSecond(); snapshot.totalSupply = totalSupply; } diff --git a/contracts/sfc/SFCI.sol b/contracts/sfc/SFCI.sol index 40d90f2..205fff2 100644 --- a/contracts/sfc/SFCI.sol +++ b/contracts/sfc/SFCI.sol @@ -44,6 +44,7 @@ interface SFCI { view returns ( uint256 endTime, + uint256 endBlock, uint256 epochFee, uint256 totalBaseRewardWeight, uint256 totalTxRewardWeight, @@ -137,6 +138,8 @@ interface SFCI { function getEpochOfflineBlocks(uint256 epoch, uint256 validatorID) external view returns (uint256); + function getEpochEndBlock(uint256 epoch) external view returns (uint256); + function rewardsStash(address delegator, uint256 validatorID) external view returns (uint256); function getLockedStake(address delegator, uint256 toValidatorID) external view returns (uint256); diff --git a/contracts/sfc/SFCState.sol b/contracts/sfc/SFCState.sol index ec74fdd..313dc39 100644 --- a/contracts/sfc/SFCState.sol +++ b/contracts/sfc/SFCState.sol @@ -71,6 +71,7 @@ contract SFCState is Initializable, Ownable { mapping(uint256 => uint256) offlineBlocks; uint256[] validatorIDs; uint256 endTime; + uint256 endBlock; uint256 epochFee; uint256 totalBaseRewardWeight; uint256 totalTxRewardWeight; diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol index 88374bb..263d8db 100644 --- a/contracts/test/UnitTestSFC.sol +++ b/contracts/test/UnitTestSFC.sol @@ -120,6 +120,7 @@ interface SFCUnitTestI { view returns ( uint256 endTime, + uint256 endBlock, uint256 epochFee, uint256 totalBaseRewardWeight, uint256 totalTxRewardWeight, @@ -215,6 +216,8 @@ interface SFCUnitTestI { function getEpochOfflineBlocks(uint256 epoch, uint256 validatorID) external view returns (uint256); + function getEpochEndBlock(uint256 epoch) external view returns (uint256); + function rewardsStash(address delegator, uint256 validatorID) external view returns (uint256); function getLockedStake(address delegator, uint256 toValidatorID) external view returns (uint256); diff --git a/test/SFC.ts b/test/SFC.ts index 2df1df6..f00b9ba 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -413,6 +413,16 @@ describe('SFC', () => { expect(await this.sfc.currentEpoch.call()).to.equal(6); expect(await this.sfc.currentSealedEpoch()).to.equal(5); }); + + it('Should succeed and return endBlock', async function () { + const epochNumber = await this.sfc.currentEpoch(); + await this.sfc.enableNonNodeCalls(); + await this.sfc.sealEpoch([100, 101, 102], [100, 101, 102], [100, 101, 102], [100, 101, 102], 0); + const lastBlock = await ethers.provider.getBlockNumber(); + // endBlock is on second position + expect((await this.sfc.getEpochSnapshot(epochNumber))[1]).to.equal(lastBlock); + expect(await this.sfc.getEpochEndBlock(epochNumber)).to.equal(lastBlock); + }); }); }); From 78101a8e2cae20e20349b22a5a77e665b023f6a5 Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 18:45:04 +0200 Subject: [PATCH 4/7] Add custom errors for `common` and `ownership` packages (#73) --- contracts/common/Initializable.sol | 9 ++++++++- contracts/common/ReentrancyGuard.sol | 11 +++++++++-- contracts/ownership/Ownable.sol | 21 +++++++++++++++++---- test/NodeDriver.ts | 21 ++++++++++++--------- test/SFC.ts | 11 ++++++----- 5 files changed, 52 insertions(+), 21 deletions(-) diff --git a/contracts/common/Initializable.sol b/contracts/common/Initializable.sol index af84fbe..905d424 100644 --- a/contracts/common/Initializable.sol +++ b/contracts/common/Initializable.sol @@ -24,11 +24,18 @@ contract Initializable { */ bool private initializing; + /** + * @dev The contract instance has already been initialized. + */ + error ContractInitialized(); + /** * @dev Modifier to use in the initializer function of a contract. */ modifier initializer() { - require(initializing || !initialized, "Contract instance has already been initialized"); + if (!initializing && initialized) { + revert ContractInitialized(); + } bool isTopLevelCall = !initializing; if (isTopLevelCall) { diff --git a/contracts/common/ReentrancyGuard.sol b/contracts/common/ReentrancyGuard.sol index 8d1d296..3c7f1d2 100644 --- a/contracts/common/ReentrancyGuard.sol +++ b/contracts/common/ReentrancyGuard.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./Initializable.sol"; +import {Initializable} from "./Initializable.sol"; /** * @dev Contract module that helps prevent reentrant calls to a function. @@ -19,6 +19,11 @@ contract ReentrancyGuard is Initializable { // counter to allow mutex lock with only one SSTORE operation uint256 private _guardCounter; + /** + * @dev Reentrant call. + */ + error ReentrantCall(); + function initialize() internal initializer { // The counter starts at one to prevent changing it from zero to a non-zero // value, which is a more expensive operation. @@ -36,7 +41,9 @@ contract ReentrancyGuard is Initializable { _guardCounter += 1; uint256 localCounter = _guardCounter; _; - require(localCounter == _guardCounter, "ReentrancyGuard: reentrant call"); + if (localCounter != _guardCounter) { + revert ReentrantCall(); + } } uint256[50] private ______gap; diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index 3466077..85780ef 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -1,8 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../common/Initializable.sol"; - +import {Initializable} from "../common/Initializable.sol"; /** * @dev Contract module which provides a basic access control mechanism, where * there is an account (an owner) that can be granted exclusive access to @@ -15,6 +14,16 @@ import "../common/Initializable.sol"; contract Ownable is Initializable { address private _owner; + /** + * @dev The caller account is not authorized to perform an operation. + */ + error OwnableUnauthorizedAccount(address account); + + /** + * @dev The owner is not a valid owner account. (eg. `address(0)`) + */ + error OwnableInvalidOwner(address owner); + event OwnershipTransferred(address indexed previousOwner, address indexed newOwner); /** @@ -36,7 +45,9 @@ contract Ownable is Initializable { * @dev Throws if called by any account other than the owner. */ modifier onlyOwner() { - require(isOwner(), "Ownable: caller is not the owner"); + if (!isOwner()) { + revert OwnableUnauthorizedAccount(msg.sender); + } _; } @@ -71,7 +82,9 @@ contract Ownable is Initializable { * @dev Transfers ownership of the contract to a new account (`newOwner`). */ function _transferOwnership(address newOwner) internal { - require(newOwner != address(0), "Ownable: new owner is the zero address"); + if (newOwner == address(0)) { + revert OwnableInvalidOwner(address(0)); + } emit OwnershipTransferred(_owner, newOwner); _owner = newOwner; } diff --git a/test/NodeDriver.ts b/test/NodeDriver.ts index 87bb017..937d2bd 100644 --- a/test/NodeDriver.ts +++ b/test/NodeDriver.ts @@ -38,8 +38,9 @@ describe('NodeDriver', () => { it('Should revert when not owner', async function () { const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).migrateTo(account)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); @@ -52,9 +53,9 @@ describe('NodeDriver', () => { it('Should revert when not owner', async function () { const address = ethers.Wallet.createRandom(); - await expect(this.nodeDriverAuth.connect(this.nonOwner).copyCode(this.sfc, address)).to.be.revertedWith( - 'Ownable: caller is not the owner', - ); + await expect( + this.nodeDriverAuth.connect(this.nonOwner).copyCode(this.sfc, address), + ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableUnauthorizedAccount'); }); }); @@ -66,8 +67,9 @@ describe('NodeDriver', () => { }); it('Should revert when not owner', async function () { - await expect(this.nodeDriverAuth.connect(this.nonOwner).updateNetworkVersion(1)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).updateNetworkVersion(1)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); @@ -78,8 +80,9 @@ describe('NodeDriver', () => { }); it('Should revert when not owner', async function () { - await expect(this.nodeDriverAuth.connect(this.nonOwner).advanceEpochs(10)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.nodeDriverAuth.connect(this.nonOwner).advanceEpochs(10)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); }); diff --git a/test/SFC.ts b/test/SFC.ts index f00b9ba..d21cc0a 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -290,15 +290,16 @@ describe('SFC', () => { }); it('Should revert when transferring ownership if not owner', async function () { - await expect(this.sfc.connect(this.user).transferOwnership(ethers.ZeroAddress)).to.be.revertedWith( - 'Ownable: caller is not the owner', + await expect(this.sfc.connect(this.user).transferOwnership(ethers.ZeroAddress)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'OwnableUnauthorizedAccount', ); }); it('Should revert when transferring ownership to zero address', async function () { - await expect(this.sfc.transferOwnership(ethers.ZeroAddress)).to.be.revertedWith( - 'Ownable: new owner is the zero address', - ); + await expect(this.sfc.transferOwnership(ethers.ZeroAddress)) + .to.be.revertedWithCustomError(this.nodeDriverAuth, 'OwnableInvalidOwner') + .withArgs(ethers.ZeroAddress); }); }); From 7be31eeca41844cff8a032b030bbbfd8e6984b1d Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 18:50:44 +0200 Subject: [PATCH 5/7] Add custom errors for `ConstantsManager` (#75) --- contracts/sfc/ConstantsManager.sol | 122 +++++++++++++++++++++------- contracts/sfc/GasPriceConstants.sol | 2 +- 2 files changed, 94 insertions(+), 30 deletions(-) diff --git a/contracts/sfc/ConstantsManager.sol b/contracts/sfc/ConstantsManager.sol index 7a7b8d3..b5283ea 100644 --- a/contracts/sfc/ConstantsManager.sol +++ b/contracts/sfc/ConstantsManager.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../ownership/Ownable.sol"; -import "../common/Decimal.sol"; +import {Ownable} from "../ownership/Ownable.sol"; +import {Decimal} from "../common/Decimal.sol"; contract ConstantsManager is Ownable { // Minimum amount of stake for a validator, i.e., 500000 FTM @@ -32,94 +32,158 @@ contract ConstantsManager is Ownable { uint256 public targetGasPowerPerSecond; uint256 public gasPriceBalancingCounterweight; + /** + * @dev Given value is too small + */ + error ValueTooSmall(); + + /** + * @dev Given value is too large + */ + error ValueTooLarge(); + function initialize() external initializer { Ownable.initialize(msg.sender); } function updateMinSelfStake(uint256 v) external virtual onlyOwner { - require(v >= 100000 * 1e18, "too small value"); - require(v <= 10000000 * 1e18, "too large value"); + if (v < 100000 * 1e18) { + revert ValueTooSmall(); + } + if (v > 10000000 * 1e18) { + revert ValueTooLarge(); + } minSelfStake = v; } function updateMaxDelegatedRatio(uint256 v) external virtual onlyOwner { - require(v >= Decimal.unit(), "too small value"); - require(v <= 31 * Decimal.unit(), "too large value"); + if (v < Decimal.unit()) { + revert ValueTooSmall(); + } + if (v > 31 * Decimal.unit()) { + revert ValueTooLarge(); + } maxDelegatedRatio = v; } function updateValidatorCommission(uint256 v) external virtual onlyOwner { - require(v <= Decimal.unit() / 2, "too large value"); + if (v > Decimal.unit() / 2) { + revert ValueTooLarge(); + } validatorCommission = v; } function updateBurntFeeShare(uint256 v) external virtual onlyOwner { - require(v <= Decimal.unit() / 2, "too large value"); + if (v > Decimal.unit() / 2) { + revert ValueTooLarge(); + } burntFeeShare = v; } function updateTreasuryFeeShare(uint256 v) external virtual onlyOwner { - require(v <= Decimal.unit() / 2, "too large value"); + if (v > Decimal.unit() / 2) { + revert ValueTooLarge(); + } treasuryFeeShare = v; } function updateUnlockedRewardRatio(uint256 v) external virtual onlyOwner { - require(v >= (5 * Decimal.unit()) / 100, "too small value"); - require(v <= Decimal.unit() / 2, "too large value"); + if (v < (5 * Decimal.unit()) / 100) { + revert ValueTooSmall(); + } + if (v > Decimal.unit() / 2) { + revert ValueTooLarge(); + } unlockedRewardRatio = v; } function updateMinLockupDuration(uint256 v) external virtual onlyOwner { - require(v >= 86400, "too small value"); - require(v <= 86400 * 30, "too large value"); + if (v < 86400) { + revert ValueTooSmall(); + } + if (v > 86400 * 30) { + revert ValueTooLarge(); + } minLockupDuration = v; } function updateMaxLockupDuration(uint256 v) external virtual onlyOwner { - require(v >= 86400 * 30, "too small value"); - require(v <= 86400 * 1460, "too large value"); + if (v < 86400 * 30) { + revert ValueTooSmall(); + } + if (v > 86400 * 1460) { + revert ValueTooLarge(); + } maxLockupDuration = v; } function updateWithdrawalPeriodEpochs(uint256 v) external virtual onlyOwner { - require(v >= 2, "too small value"); - require(v <= 100, "too large value"); + if (v < 2) { + revert ValueTooSmall(); + } + if (v > 100) { + revert ValueTooLarge(); + } withdrawalPeriodEpochs = v; } function updateWithdrawalPeriodTime(uint256 v) external virtual onlyOwner { - require(v >= 86400, "too small value"); - require(v <= 30 * 86400, "too large value"); + if (v < 86400) { + revert ValueTooSmall(); + } + if (v > 30 * 86400) { + revert ValueTooLarge(); + } withdrawalPeriodTime = v; } function updateBaseRewardPerSecond(uint256 v) external virtual onlyOwner { - require(v >= 0.5 * 1e18, "too small value"); - require(v <= 32 * 1e18, "too large value"); + if (v < 0.5 * 1e18) { + revert ValueTooSmall(); + } + if (v > 32 * 1e18) { + revert ValueTooLarge(); + } baseRewardPerSecond = v; } function updateOfflinePenaltyThresholdTime(uint256 v) external virtual onlyOwner { - require(v >= 86400, "too small value"); - require(v <= 10 * 86400, "too large value"); + if (v < 86400) { + revert ValueTooSmall(); + } + if (v > 10 * 86400) { + revert ValueTooLarge(); + } offlinePenaltyThresholdTime = v; } function updateOfflinePenaltyThresholdBlocksNum(uint256 v) external virtual onlyOwner { - require(v >= 100, "too small value"); - require(v <= 1000000, "too large value"); + if (v < 100) { + revert ValueTooSmall(); + } + if (v > 1000000) { + revert ValueTooLarge(); + } offlinePenaltyThresholdBlocksNum = v; } function updateTargetGasPowerPerSecond(uint256 v) external virtual onlyOwner { - require(v >= 1000000, "too small value"); - require(v <= 500000000, "too large value"); + if (v < 1000000) { + revert ValueTooSmall(); + } + if (v > 500000000) { + revert ValueTooLarge(); + } targetGasPowerPerSecond = v; } function updateGasPriceBalancingCounterweight(uint256 v) external virtual onlyOwner { - require(v >= 100, "too small value"); - require(v <= 10 * 86400, "too large value"); + if (v < 100) { + revert ValueTooSmall(); + } + if (v > 10 * 86400) { + revert ValueTooLarge(); + } gasPriceBalancingCounterweight = v; } } diff --git a/contracts/sfc/GasPriceConstants.sol b/contracts/sfc/GasPriceConstants.sol index 96d5931..2bc7be3 100644 --- a/contracts/sfc/GasPriceConstants.sol +++ b/contracts/sfc/GasPriceConstants.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../common/Decimal.sol"; +import {Decimal} from "../common/Decimal.sol"; library GP { function trimGasPriceChangeRatio(uint256 x) internal pure returns (uint256) { From 15407b97ebce0d1e4e7eeed58a3b5c48ec8880ce Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Thu, 24 Oct 2024 10:17:55 +0200 Subject: [PATCH 6/7] Fix solhint warnings (#76) --- .prettierrc | 3 +- .solhint.json | 4 +- .solhintignore | 1 + contracts/interfaces/IEVMWriter.sol | 14 ++ contracts/sfc/Migrations.sol | 21 +- contracts/sfc/NetworkInitializer.sol | 8 +- contracts/sfc/NodeDriver.sol | 244 +------------------- contracts/sfc/NodeDriverAuth.sol | 229 ++++++++++++++++++ contracts/sfc/SFC.sol | 31 +-- contracts/sfc/SFCBase.sol | 4 +- contracts/sfc/SFCLib.sol | 25 +- contracts/sfc/SFCState.sol | 7 +- contracts/sfc/Updater.sol | 9 +- contracts/test/StubEvmWriter.sol | 4 +- contracts/test/UnitTestConstantsManager.sol | 3 +- contracts/test/UnitTestSFC.sol | 11 +- 16 files changed, 326 insertions(+), 292 deletions(-) create mode 100644 .solhintignore create mode 100644 contracts/interfaces/IEVMWriter.sol create mode 100644 contracts/sfc/NodeDriverAuth.sol diff --git a/.prettierrc b/.prettierrc index 7e7ac00..3a9c8a9 100644 --- a/.prettierrc +++ b/.prettierrc @@ -7,8 +7,7 @@ { "files": "*.sol", "options": { - "singleQuote": false, - "quoteProps": "consistent" + "singleQuote": false } } ], diff --git a/.solhint.json b/.solhint.json index 38d8b19..856919e 100644 --- a/.solhint.json +++ b/.solhint.json @@ -1,6 +1,8 @@ { "extends": "solhint:recommended", "rules": { - "max-line-length": "off" + "func-visibility": ["warn", {"ignoreConstructors": true}], + "max-states-count": ["off"], + "no-inline-assembly": "off" } } diff --git a/.solhintignore b/.solhintignore new file mode 100644 index 0000000..57a8c75 --- /dev/null +++ b/.solhintignore @@ -0,0 +1 @@ +contracts/test \ No newline at end of file diff --git a/contracts/interfaces/IEVMWriter.sol b/contracts/interfaces/IEVMWriter.sol new file mode 100644 index 0000000..575a7b4 --- /dev/null +++ b/contracts/interfaces/IEVMWriter.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.9; + +interface IEvmWriter { + function setBalance(address acc, uint256 value) external; + + function copyCode(address acc, address from) external; + + function swapCode(address acc, address where) external; + + function setStorage(address acc, bytes32 key, bytes32 value) external; + + function incNonce(address acc, uint256 diff) external; +} diff --git a/contracts/sfc/Migrations.sol b/contracts/sfc/Migrations.sol index a9f9b98..3b5d39f 100644 --- a/contracts/sfc/Migrations.sol +++ b/contracts/sfc/Migrations.sol @@ -3,23 +3,30 @@ pragma solidity ^0.8.9; contract Migrations { address public owner; - uint public last_completed_migration; + uint256 public lastCompletedMigration; + + /** + * @dev The caller is not the owner. + */ + error NotOwner(); constructor(address contractOwner) { owner = contractOwner; } modifier restricted() { - require(msg.sender == owner, "Not the contract owner"); + if (msg.sender != owner) { + revert NotOwner(); + } _; } - function setCompleted(uint completed) public restricted { - last_completed_migration = completed; + function setCompleted(uint256 completed) public restricted { + lastCompletedMigration = completed; } - function upgrade(address new_address) public restricted { - Migrations upgraded = Migrations(new_address); - upgraded.setCompleted(last_completed_migration); + function upgrade(address newAddress) public restricted { + Migrations upgraded = Migrations(newAddress); + upgraded.setCompleted(lastCompletedMigration); } } diff --git a/contracts/sfc/NetworkInitializer.sol b/contracts/sfc/NetworkInitializer.sol index d57b3b2..51d6b5a 100644 --- a/contracts/sfc/NetworkInitializer.sol +++ b/contracts/sfc/NetworkInitializer.sol @@ -1,10 +1,10 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./SFCI.sol"; -import "./NodeDriver.sol"; -import "./SFCLib.sol"; -import "./ConstantsManager.sol"; +import {SFCI} from "./SFCI.sol"; +import {NodeDriver, NodeDriverAuth} from "./NodeDriver.sol"; +import {ConstantsManager} from "./ConstantsManager.sol"; +import {Decimal} from "../common/Decimal.sol"; contract NetworkInitializer { // Initialize NodeDriverAuth, NodeDriver and SFC in one call to allow fewer genesis transactions diff --git a/contracts/sfc/NodeDriver.sol b/contracts/sfc/NodeDriver.sol index 5bfe390..09090fa 100644 --- a/contracts/sfc/NodeDriver.sol +++ b/contracts/sfc/NodeDriver.sol @@ -1,235 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../common/Initializable.sol"; -import "../ownership/Ownable.sol"; -import "./SFCI.sol"; - -interface NodeDriverExecutable { - function execute() external; -} - -contract NodeDriverAuth is Initializable, Ownable { - SFCI internal sfc; - NodeDriver internal driver; - - // Initialize NodeDriverAuth, NodeDriver and SFC in one call to allow fewer genesis transactions - function initialize(address payable _sfc, address _driver, address _owner) external initializer { - Ownable.initialize(_owner); - driver = NodeDriver(_driver); - sfc = SFCI(_sfc); - } - - modifier onlySFC() { - require(msg.sender == address(sfc), "caller is not the SFC contract"); - _; - } - - modifier onlyDriver() { - require(msg.sender == address(driver), "caller is not the NodeDriver contract"); - _; - } - - function migrateTo(address newDriverAuth) external onlyOwner { - driver.setBackend(newDriverAuth); - } - - function _execute(address executable, address newOwner, bytes32 selfCodeHash, bytes32 driverCodeHash) internal { - _transferOwnership(executable); - NodeDriverExecutable(executable).execute(); - _transferOwnership(newOwner); - //require(driver.backend() == address(this), "ownership of driver is lost"); - require(_getCodeHash(address(this)) == selfCodeHash, "self code hash doesn't match"); - require(_getCodeHash(address(driver)) == driverCodeHash, "driver code hash doesn't match"); - } - - function execute(address executable) external onlyOwner { - _execute(executable, owner(), _getCodeHash(address(this)), _getCodeHash(address(driver))); - } - - function mutExecute( - address executable, - address newOwner, - bytes32 selfCodeHash, - bytes32 driverCodeHash - ) external onlyOwner { - _execute(executable, newOwner, selfCodeHash, driverCodeHash); - } - - function incBalance(address acc, uint256 diff) external onlySFC { - require(acc == address(sfc), "recipient is not the SFC contract"); - driver.setBalance(acc, address(acc).balance + diff); - } - - function upgradeCode(address acc, address from) external onlyOwner { - require(isContract(acc) && isContract(from), "not a contract"); - driver.copyCode(acc, from); - } - - function copyCode(address acc, address from) external onlyOwner { - driver.copyCode(acc, from); - } - - function incNonce(address acc, uint256 diff) external onlyOwner { - driver.incNonce(acc, diff); - } - - function updateNetworkRules(bytes calldata diff) external onlyOwner { - driver.updateNetworkRules(diff); - } - - function updateMinGasPrice(uint256 minGasPrice) external onlySFC { - // prettier-ignore - driver.updateNetworkRules(bytes(strConcat("{\"Economy\":{\"MinGasPrice\":", uint256ToStr(minGasPrice), "}}"))); - } - - function updateNetworkVersion(uint256 version) external onlyOwner { - driver.updateNetworkVersion(version); - } - - function advanceEpochs(uint256 num) external onlyOwner { - driver.advanceEpochs(num); - } - - function updateValidatorWeight(uint256 validatorID, uint256 value) external onlySFC { - driver.updateValidatorWeight(validatorID, value); - } - - function updateValidatorPubkey(uint256 validatorID, bytes calldata pubkey) external onlySFC { - driver.updateValidatorPubkey(validatorID, pubkey); - } - - function setGenesisValidator( - address _auth, - uint256 validatorID, - bytes calldata pubkey, - uint256 status, - uint256 createdEpoch, - uint256 createdTime, - uint256 deactivatedEpoch, - uint256 deactivatedTime - ) external onlyDriver { - sfc.setGenesisValidator( - _auth, - validatorID, - pubkey, - status, - createdEpoch, - createdTime, - deactivatedEpoch, - deactivatedTime - ); - } - - function setGenesisDelegation( - address delegator, - uint256 toValidatorID, - uint256 stake, - uint256 lockedStake, - uint256 lockupFromEpoch, - uint256 lockupEndTime, - uint256 lockupDuration, - uint256 earlyUnlockPenalty, - uint256 rewards - ) external onlyDriver { - sfc.setGenesisDelegation( - delegator, - toValidatorID, - stake, - lockedStake, - lockupFromEpoch, - lockupEndTime, - lockupDuration, - earlyUnlockPenalty, - rewards - ); - } - - function deactivateValidator(uint256 validatorID, uint256 status) external onlyDriver { - sfc.deactivateValidator(validatorID, status); - } - - function sealEpochValidators(uint256[] calldata nextValidatorIDs) external onlyDriver { - sfc.sealEpochValidators(nextValidatorIDs); - } - - function sealEpoch( - uint256[] calldata offlineTimes, - uint256[] calldata offlineBlocks, - uint256[] calldata uptimes, - uint256[] calldata originatedTxsFee, - uint256 usedGas - ) external onlyDriver { - sfc.sealEpoch(offlineTimes, offlineBlocks, uptimes, originatedTxsFee, usedGas); - } - - function isContract(address account) internal view returns (bool) { - uint256 size; - // solhint-disable-next-line no-inline-assembly - assembly { - size := extcodesize(account) - } - return size > 0; - } - - function decimalsNum(uint256 num) internal pure returns (uint256) { - uint decimals; - while (num != 0) { - decimals++; - num /= 10; - } - return decimals; - } - - function uint256ToStr(uint256 num) internal pure returns (string memory) { - if (num == 0) { - return "0"; - } - uint decimals = decimalsNum(num); - bytes memory bstr = new bytes(decimals); - uint strIdx = decimals - 1; - while (num != 0) { - bstr[strIdx] = bytes1(uint8(48 + (num % 10))); - num /= 10; - if (strIdx > 0) { - strIdx--; - } - } - return string(bstr); - } - - function strConcat(string memory _a, string memory _b, string memory _c) internal pure returns (string memory) { - bytes memory _ba = bytes(_a); - bytes memory _bb = bytes(_b); - bytes memory _bc = bytes(_c); - string memory abc = new string(_ba.length + _bb.length + _bc.length); - bytes memory babc = bytes(abc); - uint k = 0; - uint i = 0; - for (i = 0; i < _ba.length; i++) { - babc[k++] = _ba[i]; - } - for (i = 0; i < _bb.length; i++) { - babc[k++] = _bb[i]; - } - for (i = 0; i < _bc.length; i++) { - babc[k++] = _bc[i]; - } - return string(babc); - } - - function _getCodeHash(address addr) internal view returns (bytes32) { - bytes32 codeHash; - assembly { - codeHash := extcodehash(addr) - } - return codeHash; - } -} +import {Initializable} from "../common/Initializable.sol"; +import {NodeDriverAuth} from "./NodeDriverAuth.sol"; +import {IEvmWriter} from "../interfaces/IEVMWriter.sol"; contract NodeDriver is Initializable { NodeDriverAuth internal backend; - EVMWriter internal evmWriter; + IEvmWriter internal evmWriter; event UpdatedBackend(address indexed backend); @@ -253,7 +31,7 @@ contract NodeDriver is Initializable { function initialize(address _backend, address _evmWriterAddress) external initializer { backend = NodeDriverAuth(_backend); emit UpdatedBackend(_backend); - evmWriter = EVMWriter(_evmWriterAddress); + evmWriter = IEvmWriter(_evmWriterAddress); } function setBalance(address acc, uint256 value) external onlyBackend { @@ -376,15 +154,3 @@ contract NodeDriver is Initializable { backend.sealEpoch(offlineTimes, offlineBlocks, uptimes, originatedTxsFee, usedGas); } } - -interface EVMWriter { - function setBalance(address acc, uint256 value) external; - - function copyCode(address acc, address from) external; - - function swapCode(address acc, address where) external; - - function setStorage(address acc, bytes32 key, bytes32 value) external; - - function incNonce(address acc, uint256 diff) external; -} diff --git a/contracts/sfc/NodeDriverAuth.sol b/contracts/sfc/NodeDriverAuth.sol new file mode 100644 index 0000000..f0362b7 --- /dev/null +++ b/contracts/sfc/NodeDriverAuth.sol @@ -0,0 +1,229 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity ^0.8.9; + +import {Initializable} from "../common/Initializable.sol"; +import {Ownable} from "../ownership/Ownable.sol"; +import {SFCI} from "./SFCI.sol"; +import {NodeDriver} from "./NodeDriver.sol"; + +interface NodeDriverExecutable { + function execute() external; +} + +contract NodeDriverAuth is Initializable, Ownable { + SFCI internal sfc; + NodeDriver internal driver; + + // Initialize NodeDriverAuth, NodeDriver and SFC in one call to allow fewer genesis transactions + function initialize(address payable _sfc, address _driver, address _owner) external initializer { + Ownable.initialize(_owner); + driver = NodeDriver(_driver); + sfc = SFCI(_sfc); + } + + modifier onlySFC() { + require(msg.sender == address(sfc), "caller is not the SFC contract"); + _; + } + + modifier onlyDriver() { + require(msg.sender == address(driver), "caller is not the NodeDriver contract"); + _; + } + + function migrateTo(address newDriverAuth) external onlyOwner { + driver.setBackend(newDriverAuth); + } + + function _execute(address executable, address newOwner, bytes32 selfCodeHash, bytes32 driverCodeHash) internal { + _transferOwnership(executable); + NodeDriverExecutable(executable).execute(); + _transferOwnership(newOwner); + //require(driver.backend() == address(this), "ownership of driver is lost"); + require(_getCodeHash(address(this)) == selfCodeHash, "self code hash doesn't match"); + require(_getCodeHash(address(driver)) == driverCodeHash, "driver code hash doesn't match"); + } + + function execute(address executable) external onlyOwner { + _execute(executable, owner(), _getCodeHash(address(this)), _getCodeHash(address(driver))); + } + + function mutExecute( + address executable, + address newOwner, + bytes32 selfCodeHash, + bytes32 driverCodeHash + ) external onlyOwner { + _execute(executable, newOwner, selfCodeHash, driverCodeHash); + } + + function incBalance(address acc, uint256 diff) external onlySFC { + require(acc == address(sfc), "recipient is not the SFC contract"); + driver.setBalance(acc, address(acc).balance + diff); + } + + function upgradeCode(address acc, address from) external onlyOwner { + require(isContract(acc) && isContract(from), "not a contract"); + driver.copyCode(acc, from); + } + + function copyCode(address acc, address from) external onlyOwner { + driver.copyCode(acc, from); + } + + function incNonce(address acc, uint256 diff) external onlyOwner { + driver.incNonce(acc, diff); + } + + function updateNetworkRules(bytes calldata diff) external onlyOwner { + driver.updateNetworkRules(diff); + } + + function updateMinGasPrice(uint256 minGasPrice) external onlySFC { + // prettier-ignore + driver.updateNetworkRules(bytes(strConcat("{\"Economy\":{\"MinGasPrice\":", uint256ToStr(minGasPrice), "}}"))); + } + + function updateNetworkVersion(uint256 version) external onlyOwner { + driver.updateNetworkVersion(version); + } + + function advanceEpochs(uint256 num) external onlyOwner { + driver.advanceEpochs(num); + } + + function updateValidatorWeight(uint256 validatorID, uint256 value) external onlySFC { + driver.updateValidatorWeight(validatorID, value); + } + + function updateValidatorPubkey(uint256 validatorID, bytes calldata pubkey) external onlySFC { + driver.updateValidatorPubkey(validatorID, pubkey); + } + + function setGenesisValidator( + address _auth, + uint256 validatorID, + bytes calldata pubkey, + uint256 status, + uint256 createdEpoch, + uint256 createdTime, + uint256 deactivatedEpoch, + uint256 deactivatedTime + ) external onlyDriver { + sfc.setGenesisValidator( + _auth, + validatorID, + pubkey, + status, + createdEpoch, + createdTime, + deactivatedEpoch, + deactivatedTime + ); + } + + function setGenesisDelegation( + address delegator, + uint256 toValidatorID, + uint256 stake, + uint256 lockedStake, + uint256 lockupFromEpoch, + uint256 lockupEndTime, + uint256 lockupDuration, + uint256 earlyUnlockPenalty, + uint256 rewards + ) external onlyDriver { + sfc.setGenesisDelegation( + delegator, + toValidatorID, + stake, + lockedStake, + lockupFromEpoch, + lockupEndTime, + lockupDuration, + earlyUnlockPenalty, + rewards + ); + } + + function deactivateValidator(uint256 validatorID, uint256 status) external onlyDriver { + sfc.deactivateValidator(validatorID, status); + } + + function sealEpochValidators(uint256[] calldata nextValidatorIDs) external onlyDriver { + sfc.sealEpochValidators(nextValidatorIDs); + } + + function sealEpoch( + uint256[] calldata offlineTimes, + uint256[] calldata offlineBlocks, + uint256[] calldata uptimes, + uint256[] calldata originatedTxsFee, + uint256 usedGas + ) external onlyDriver { + sfc.sealEpoch(offlineTimes, offlineBlocks, uptimes, originatedTxsFee, usedGas); + } + + function isContract(address account) internal view returns (bool) { + uint256 size; + // solhint-disable-next-line no-inline-assembly + assembly { + size := extcodesize(account) + } + return size > 0; + } + + function decimalsNum(uint256 num) internal pure returns (uint256) { + uint256 decimals; + while (num != 0) { + decimals++; + num /= 10; + } + return decimals; + } + + function uint256ToStr(uint256 num) internal pure returns (string memory) { + if (num == 0) { + return "0"; + } + uint256 decimals = decimalsNum(num); + bytes memory bstr = new bytes(decimals); + uint256 strIdx = decimals - 1; + while (num != 0) { + bstr[strIdx] = bytes1(uint8(48 + (num % 10))); + num /= 10; + if (strIdx > 0) { + strIdx--; + } + } + return string(bstr); + } + + function strConcat(string memory _a, string memory _b, string memory _c) internal pure returns (string memory) { + bytes memory _ba = bytes(_a); + bytes memory _bb = bytes(_b); + bytes memory _bc = bytes(_c); + string memory abc = new string(_ba.length + _bb.length + _bc.length); + bytes memory babc = bytes(abc); + uint256 k = 0; + uint256 i = 0; + for (i = 0; i < _ba.length; i++) { + babc[k++] = _ba[i]; + } + for (i = 0; i < _bb.length; i++) { + babc[k++] = _bb[i]; + } + for (i = 0; i < _bc.length; i++) { + babc[k++] = _bc[i]; + } + return string(babc); + } + + function _getCodeHash(address addr) internal view returns (bytes32) { + bytes32 codeHash; + assembly { + codeHash := extcodehash(addr) + } + return codeHash; + } +} diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index 0f7e31d..3b61b85 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -1,9 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./GasPriceConstants.sol"; -import "../version/Version.sol"; -import "./SFCBase.sol"; +import {Ownable} from "../ownership/Ownable.sol"; +import {Decimal} from "../common/Decimal.sol"; +import {SFCBase} from "./SFCBase.sol"; +import {NodeDriverAuth} from "./NodeDriverAuth.sol"; +import {ConstantsManager} from "./ConstantsManager.sol"; +import {GP} from "./GasPriceConstants.sol"; +import {Version} from "../version/Version.sol"; /** * @dev Stakers contract defines data structure and methods for validators / validators. @@ -34,6 +38,7 @@ contract SFC is SFCBase, Version { } } + // solhint-disable-next-line no-complex-fallback fallback() external payable { require(msg.data.length != 0, "transfers not allowed"); _delegate(libAddress); @@ -185,7 +190,7 @@ contract SFC is SFCBase, Version { Epoch callbacks */ - function _sealEpoch_offline( + function _sealEpochOffline( EpochSnapshot storage snapshot, uint256[] memory validatorIDs, uint256[] memory offlineTime, @@ -206,7 +211,7 @@ contract SFC is SFCBase, Version { } } - struct _SealEpochRewardsCtx { + struct SealEpochRewardsCtx { uint256[] baseRewardWeights; uint256 totalBaseRewardWeight; uint256[] txRewardWeights; @@ -214,7 +219,7 @@ contract SFC is SFCBase, Version { uint256 epochFee; } - function _sealEpoch_rewards( + function _sealEpochRewards( uint256 epochDuration, EpochSnapshot storage snapshot, EpochSnapshot storage prevSnapshot, @@ -222,10 +227,10 @@ contract SFC is SFCBase, Version { uint256[] memory uptimes, uint256[] memory accumulatedOriginatedTxsFee ) internal { - _SealEpochRewardsCtx memory ctx = _SealEpochRewardsCtx( - new uint[](validatorIDs.length), + SealEpochRewardsCtx memory ctx = SealEpochRewardsCtx( + new uint256[](validatorIDs.length), 0, - new uint[](validatorIDs.length), + new uint256[](validatorIDs.length), 0, 0 ); @@ -322,7 +327,7 @@ contract SFC is SFCBase, Version { } } - function _sealEpoch_minGasPrice(uint256 epochDuration, uint256 epochGas) internal { + function _sealEpochMinGasPrice(uint256 epochDuration, uint256 epochGas) internal { // change minGasPrice proportionally to the difference between target and received epochGas uint256 targetEpochGas = epochDuration * c.targetGasPowerPerSecond() + 1; uint256 gasPriceDeltaRatio = (epochGas * Decimal.unit()) / targetEpochGas; @@ -352,15 +357,15 @@ contract SFC is SFCBase, Version { EpochSnapshot storage snapshot = getEpochSnapshot[currentEpoch()]; uint256[] memory validatorIDs = snapshot.validatorIDs; - _sealEpoch_offline(snapshot, validatorIDs, offlineTime, offlineBlocks); + _sealEpochOffline(snapshot, validatorIDs, offlineTime, offlineBlocks); { EpochSnapshot storage prevSnapshot = getEpochSnapshot[currentSealedEpoch]; uint256 epochDuration = 1; if (_now() > prevSnapshot.endTime) { epochDuration = _now() - prevSnapshot.endTime; } - _sealEpoch_rewards(epochDuration, snapshot, prevSnapshot, validatorIDs, uptimes, originatedTxsFee); - _sealEpoch_minGasPrice(epochDuration, epochGas); + _sealEpochRewards(epochDuration, snapshot, prevSnapshot, validatorIDs, uptimes, originatedTxsFee); + _sealEpochMinGasPrice(epochDuration, epochGas); } currentSealedEpoch = currentEpoch(); diff --git a/contracts/sfc/SFCBase.sol b/contracts/sfc/SFCBase.sol index 066a3e4..d76acf1 100644 --- a/contracts/sfc/SFCBase.sol +++ b/contracts/sfc/SFCBase.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./SFCState.sol"; +import {Decimal} from "../common/Decimal.sol"; +import {SFCState} from "./SFCState.sol"; contract SFCBase is SFCState { uint256 internal constant OK_STATUS = 0; @@ -92,6 +93,7 @@ contract SFCBase is SFCState { function _recountVotes(address delegator, address validatorAuth, bool strict) internal { if (voteBookAddress != address(0)) { // Don't allow recountVotes to use up all the gas + // solhint-disable-next-line avoid-low-level-calls (bool success, ) = voteBookAddress.call{gas: 8000000}( abi.encodeWithSignature("recountVotes(address,address)", delegator, validatorAuth) ); diff --git a/contracts/sfc/SFCLib.sol b/contracts/sfc/SFCLib.sol index 231d767..3d65ff0 100644 --- a/contracts/sfc/SFCLib.sol +++ b/contracts/sfc/SFCLib.sol @@ -1,10 +1,8 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../common/Decimal.sol"; -import "./GasPriceConstants.sol"; -import "./SFCBase.sol"; -import "./NodeDriver.sol"; +import {Decimal} from "../common/Decimal.sol"; +import {SFCBase} from "./SFCBase.sol"; contract SFCLib is SFCBase { event CreatedValidator( @@ -180,6 +178,7 @@ contract SFCLib is SFCBase { } function recountVotes(address delegator, address validatorAuth, bool strict, uint256 gas) external { + // solhint-disable-next-line avoid-low-level-calls (bool success, ) = voteBookAddress.call{gas: gas}( abi.encodeWithSignature("recountVotes(address,address)", delegator, validatorAuth) ); @@ -319,21 +318,21 @@ contract SFCLib is SFCBase { // find highest epoch such that _isLockedUpAtEpoch returns true (using binary search) function _highestLockupEpoch(address delegator, uint256 validatorID) internal view returns (uint256) { - uint256 l = getLockupInfo[delegator][validatorID].fromEpoch; + uint256 fromEpoch = getLockupInfo[delegator][validatorID].fromEpoch; uint256 r = currentSealedEpoch; if (_isLockedUpAtEpoch(delegator, validatorID, r)) { return r; } - if (!_isLockedUpAtEpoch(delegator, validatorID, l)) { + if (!_isLockedUpAtEpoch(delegator, validatorID, fromEpoch)) { return 0; } - if (l > r) { + if (fromEpoch > r) { return 0; } - while (l < r) { - uint256 m = (l + r) / 2; + while (fromEpoch < r) { + uint256 m = (fromEpoch + r) / 2; if (_isLockedUpAtEpoch(delegator, validatorID, m)) { - l = m + 1; + fromEpoch = m + 1; } else { r = m; } @@ -824,15 +823,15 @@ contract SFCLib is SFCBase { if (step == 0) { return; } - uint256 RPS = (_getAvgUptime(toValidatorID, duration, step) * 2092846271) / duration; // corresponds to 6.6% APR + uint256 rps = (_getAvgUptime(toValidatorID, duration, step) * 2092846271) / duration; // corresponds to 6.6% APR uint256 selfStake = getStake[delegator][toValidatorID]; - uint256 avgFullReward = (((selfStake * RPS * duration) / 1e18) * (Decimal.unit() - c.validatorCommission())) / + uint256 avgFullReward = (((selfStake * rps * duration) / 1e18) * (Decimal.unit() - c.validatorCommission())) / Decimal.unit(); // reward for self-stake if (getValidator[toValidatorID].auth == delegator) { // reward for received portion of stake uint256 receivedStakeAvg = (_getAvgReceivedStake(toValidatorID, duration, step) * 11) / 10; - avgFullReward += (((receivedStakeAvg * RPS * duration) / 1e18) * c.validatorCommission()) / Decimal.unit(); + avgFullReward += (((receivedStakeAvg * rps * duration) / 1e18) * c.validatorCommission()) / Decimal.unit(); } avgFullReward = (avgFullReward * lockedStake) / selfStake; Rewards memory avgReward = _scaleLockupReward(avgFullReward, duration); diff --git a/contracts/sfc/SFCState.sol b/contracts/sfc/SFCState.sol index 313dc39..e19fadd 100644 --- a/contracts/sfc/SFCState.sol +++ b/contracts/sfc/SFCState.sol @@ -1,9 +1,10 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./NodeDriver.sol"; -import "../ownership/Ownable.sol"; -import "./ConstantsManager.sol"; +import {Ownable} from "../ownership/Ownable.sol"; +import {Initializable} from "../common/Initializable.sol"; +import {NodeDriverAuth} from "./NodeDriverAuth.sol"; +import {ConstantsManager} from "./ConstantsManager.sol"; contract SFCState is Initializable, Ownable { /** diff --git a/contracts/sfc/Updater.sol b/contracts/sfc/Updater.sol index 1d8140a..9b1324d 100644 --- a/contracts/sfc/Updater.sol +++ b/contracts/sfc/Updater.sol @@ -1,8 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "./NodeDriver.sol"; -import "./SFC.sol"; +import {Ownable} from "../ownership/Ownable.sol"; +import {Decimal} from "../common/Decimal.sol"; +import {NodeDriverAuth} from "./NodeDriverAuth.sol"; +import {ConstantsManager} from "./ConstantsManager.sol"; +import {SFC} from "./SFC.sol"; +import {SFCI} from "./SFCI.sol"; +import {Version} from "../version/Version.sol"; interface GovI { function upgrade(address v) external; diff --git a/contracts/test/StubEvmWriter.sol b/contracts/test/StubEvmWriter.sol index 56d3ef9..1c50617 100644 --- a/contracts/test/StubEvmWriter.sol +++ b/contracts/test/StubEvmWriter.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../sfc/NodeDriver.sol"; +import {IEvmWriter} from "../interfaces/IEVMWriter.sol"; -contract StubEvmWriter is EVMWriter { +contract StubEvmWriter is IEvmWriter { function setBalance(address acc, uint256 value) external {} function copyCode(address acc, address from) external {} diff --git a/contracts/test/UnitTestConstantsManager.sol b/contracts/test/UnitTestConstantsManager.sol index 4dbbdf7..275a8c7 100644 --- a/contracts/test/UnitTestConstantsManager.sol +++ b/contracts/test/UnitTestConstantsManager.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../sfc/ConstantsManager.sol"; + +import {ConstantsManager} from "../sfc/ConstantsManager.sol"; contract UnitTestConstantsManager is ConstantsManager { function updateMinSelfStake(uint256 v) external override onlyOwner { diff --git a/contracts/test/UnitTestSFC.sol b/contracts/test/UnitTestSFC.sol index 263d8db..ef7d24d 100644 --- a/contracts/test/UnitTestSFC.sol +++ b/contracts/test/UnitTestSFC.sol @@ -1,10 +1,13 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity ^0.8.9; -import "../sfc/SFC.sol"; -import "../sfc/SFCI.sol"; -import "../sfc/SFCLib.sol"; -import "./UnitTestConstantsManager.sol"; +import {Decimal} from "../common/Decimal.sol"; +import {SFC} from "../sfc/SFC.sol"; +import {SFCBase} from "../sfc/SFCBase.sol"; +import {SFCLib} from "../sfc/SFCLib.sol"; +import {NodeDriverAuth} from "../sfc/NodeDriverAuth.sol"; +import {NodeDriver} from "../sfc/NodeDriver.sol"; +import {UnitTestConstantsManager} from "./UnitTestConstantsManager.sol"; contract UnitTestSFCBase { uint256 internal time; From 846e556c39382d3b843ce81edab6a02c93a0a8de Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Thu, 24 Oct 2024 11:58:23 +0200 Subject: [PATCH 7/7] Add custom errors (#77) --- contracts/common/Initializable.sol | 4 +- contracts/common/ReentrancyGuard.sol | 4 +- contracts/ownership/Ownable.sol | 1 + contracts/sfc/NodeDriver.sol | 11 +- contracts/sfc/NodeDriverAuth.sol | 31 ++++- contracts/sfc/SFC.sol | 63 ++++++--- contracts/sfc/SFCBase.sol | 83 +++++++++++- contracts/sfc/SFCLib.sol | 189 +++++++++++++++++++-------- contracts/sfc/Updater.sol | 43 ++++-- test/NodeDriver.ts | 37 ++++-- test/SFC.ts | 121 ++++++++++------- 11 files changed, 422 insertions(+), 165 deletions(-) diff --git a/contracts/common/Initializable.sol b/contracts/common/Initializable.sol index 905d424..97d2ff7 100644 --- a/contracts/common/Initializable.sol +++ b/contracts/common/Initializable.sol @@ -27,14 +27,14 @@ contract Initializable { /** * @dev The contract instance has already been initialized. */ - error ContractInitialized(); + error InvalidInitialization(); /** * @dev Modifier to use in the initializer function of a contract. */ modifier initializer() { if (!initializing && initialized) { - revert ContractInitialized(); + revert InvalidInitialization(); } bool isTopLevelCall = !initializing; diff --git a/contracts/common/ReentrancyGuard.sol b/contracts/common/ReentrancyGuard.sol index 3c7f1d2..5919a27 100644 --- a/contracts/common/ReentrancyGuard.sol +++ b/contracts/common/ReentrancyGuard.sol @@ -22,7 +22,7 @@ contract ReentrancyGuard is Initializable { /** * @dev Reentrant call. */ - error ReentrantCall(); + error ReentrancyGuardReentrantCall(); function initialize() internal initializer { // The counter starts at one to prevent changing it from zero to a non-zero @@ -42,7 +42,7 @@ contract ReentrancyGuard is Initializable { uint256 localCounter = _guardCounter; _; if (localCounter != _guardCounter) { - revert ReentrantCall(); + revert ReentrancyGuardReentrantCall(); } } diff --git a/contracts/ownership/Ownable.sol b/contracts/ownership/Ownable.sol index 85780ef..a7292d7 100644 --- a/contracts/ownership/Ownable.sol +++ b/contracts/ownership/Ownable.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.9; import {Initializable} from "../common/Initializable.sol"; + /** * @dev Contract module which provides a basic access control mechanism, where * there is an account (an owner) that can be granted exclusive access to diff --git a/contracts/sfc/NodeDriver.sol b/contracts/sfc/NodeDriver.sol index 09090fa..c58e44c 100644 --- a/contracts/sfc/NodeDriver.sol +++ b/contracts/sfc/NodeDriver.sol @@ -9,6 +9,9 @@ contract NodeDriver is Initializable { NodeDriverAuth internal backend; IEvmWriter internal evmWriter; + error NotNode(); + error NotBackend(); + event UpdatedBackend(address indexed backend); function setBackend(address _backend) external onlyBackend { @@ -17,7 +20,9 @@ contract NodeDriver is Initializable { } modifier onlyBackend() { - require(msg.sender == address(backend), "caller is not the backend"); + if (msg.sender != address(backend)) { + revert NotBackend(); + } _; } @@ -75,7 +80,9 @@ contract NodeDriver is Initializable { } modifier onlyNode() { - require(msg.sender == address(0), "not callable"); + if (msg.sender != address(0)) { + revert NotNode(); + } _; } diff --git a/contracts/sfc/NodeDriverAuth.sol b/contracts/sfc/NodeDriverAuth.sol index f0362b7..2801fff 100644 --- a/contracts/sfc/NodeDriverAuth.sol +++ b/contracts/sfc/NodeDriverAuth.sol @@ -14,6 +14,13 @@ contract NodeDriverAuth is Initializable, Ownable { SFCI internal sfc; NodeDriver internal driver; + error NotSFC(); + error NotDriver(); + error NotContract(); + error SelfCodeHashMismatch(); + error DriverCodeHashMismatch(); + error RecipientNotSFC(); + // Initialize NodeDriverAuth, NodeDriver and SFC in one call to allow fewer genesis transactions function initialize(address payable _sfc, address _driver, address _owner) external initializer { Ownable.initialize(_owner); @@ -22,12 +29,16 @@ contract NodeDriverAuth is Initializable, Ownable { } modifier onlySFC() { - require(msg.sender == address(sfc), "caller is not the SFC contract"); + if (msg.sender != address(sfc)) { + revert NotSFC(); + } _; } modifier onlyDriver() { - require(msg.sender == address(driver), "caller is not the NodeDriver contract"); + if (msg.sender != address(driver)) { + revert NotDriver(); + } _; } @@ -40,8 +51,12 @@ contract NodeDriverAuth is Initializable, Ownable { NodeDriverExecutable(executable).execute(); _transferOwnership(newOwner); //require(driver.backend() == address(this), "ownership of driver is lost"); - require(_getCodeHash(address(this)) == selfCodeHash, "self code hash doesn't match"); - require(_getCodeHash(address(driver)) == driverCodeHash, "driver code hash doesn't match"); + if (_getCodeHash(address(this)) != selfCodeHash) { + revert SelfCodeHashMismatch(); + } + if (_getCodeHash(address(driver)) != driverCodeHash) { + revert DriverCodeHashMismatch(); + } } function execute(address executable) external onlyOwner { @@ -58,12 +73,16 @@ contract NodeDriverAuth is Initializable, Ownable { } function incBalance(address acc, uint256 diff) external onlySFC { - require(acc == address(sfc), "recipient is not the SFC contract"); + if (acc != address(sfc)) { + revert RecipientNotSFC(); + } driver.setBalance(acc, address(acc).balance + diff); } function upgradeCode(address acc, address from) external onlyOwner { - require(isContract(acc) && isContract(from), "not a contract"); + if (!isContract(acc) || !isContract(from)) { + revert NotContract(); + } driver.copyCode(acc, from); } diff --git a/contracts/sfc/SFC.sol b/contracts/sfc/SFC.sol index 3b61b85..5a3aa63 100644 --- a/contracts/sfc/SFC.sol +++ b/contracts/sfc/SFC.sol @@ -40,12 +40,14 @@ contract SFC is SFCBase, Version { // solhint-disable-next-line no-complex-fallback fallback() external payable { - require(msg.data.length != 0, "transfers not allowed"); + if (msg.data.length == 0) { + revert TransfersNotAllowed(); + } _delegate(libAddress); } receive() external payable { - revert("transfers not allowed"); + revert TransfersNotAllowed(); } /* @@ -135,24 +137,31 @@ contract SFC is SFCBase, Version { for (uint256 vid = start; vid < end; vid++) { bytes memory pubkey = getValidatorPubkey[vid]; if (pubkey.length > 0 && pubkeyHashToValidatorID[keccak256(pubkey)] != vid) { - require(pubkeyHashToValidatorID[keccak256(pubkey)] == 0, "already exists"); + if (pubkeyHashToValidatorID[keccak256(pubkey)] != 0) { + revert PubkeyExists(); + } pubkeyHashToValidatorID[keccak256(pubkey)] = vid; } } } function updateValidatorPubkey(bytes calldata pubkey) external { - require(getValidator[1].auth == 0x541E408443A592C38e01Bed0cB31f9De8c1322d0, "not mainnet"); - require(pubkey.length == 66 && pubkey[0] == 0xc0, "malformed pubkey"); + if (pubkey.length != 66 || pubkey[0] != 0xc0) { + revert MalformedPubkey(); + } uint256 validatorID = getValidatorID[msg.sender]; - require(validatorID <= 59 || validatorID == 64, "not legacy validator"); - require(_validatorExists(validatorID), "validator doesn't exist"); - require(keccak256(pubkey) != keccak256(getValidatorPubkey[validatorID]), "same pubkey"); - require(pubkeyHashToValidatorID[keccak256(pubkey)] == 0, "already used"); - require( - validatorPubkeyChanges[validatorID] == 0 || validatorID == 64 || validatorID <= 12, - "allowed only once" - ); + if (!_validatorExists(validatorID)) { + revert ValidatorNotExists(); + } + if (keccak256(pubkey) == keccak256(getValidatorPubkey[validatorID])) { + revert SamePubkey(); + } + if (pubkeyHashToValidatorID[keccak256(pubkey)] != 0) { + revert PubkeyExists(); + } + if (validatorPubkeyChanges[validatorID] != 0) { + revert PubkeyAllowedOnlyOnce(); + } validatorPubkeyChanges[validatorID]++; pubkeyHashToValidatorID[keccak256(pubkey)] = validatorID; @@ -161,7 +170,9 @@ contract SFC is SFCBase, Version { } function setRedirectionAuthorizer(address v) external onlyOwner { - require(redirectionAuthorizer != v, "same"); + if (redirectionAuthorizer == v) { + revert SameRedirectionAuthorizer(); + } redirectionAuthorizer = v; } @@ -172,16 +183,26 @@ contract SFC is SFCBase, Version { } function initiateRedirection(address from, address to) external { - require(msg.sender == redirectionAuthorizer, "not authorized"); - require(getRedirection[from] != to, "already complete"); - require(from != to, "same address"); + if (msg.sender != redirectionAuthorizer) { + revert NotAuthorized(); + } + if (getRedirection[from] == to) { + revert RequestedCompleted(); + } + if (from == to) { + revert SameAddress(); + } getRedirectionRequest[from] = to; } function redirect(address to) external { address from = msg.sender; - require(to != address(0), "zero address"); - require(getRedirectionRequest[from] == to, "no request"); + if (to == address(0)) { + revert ZeroAddress(); + } + if (getRedirectionRequest[from] != to) { + revert RequestNotExists(); + } getRedirection[from] = to; getRedirectionRequest[from] = address(0); } @@ -323,7 +344,9 @@ contract SFC is SFCBase, Version { uint256 feeShare = (ctx.epochFee * c.treasuryFeeShare()) / Decimal.unit(); _mintNativeToken(feeShare); (bool success, ) = treasuryAddress.call{value: feeShare, gas: 1000000}(""); - require(success, "treasury transfer failed"); + if (!success) { + revert TransferFailed(); + } } } diff --git a/contracts/sfc/SFCBase.sol b/contracts/sfc/SFCBase.sol index d76acf1..164404f 100644 --- a/contracts/sfc/SFCBase.sol +++ b/contracts/sfc/SFCBase.sol @@ -11,6 +11,77 @@ contract SFCBase is SFCState { uint256 internal constant DOUBLESIGN_BIT = 1 << 7; uint256 internal constant CHEATER_MASK = DOUBLESIGN_BIT; + // auth + error NotDriverAuth(); + error NotAuthorized(); + + // addresses + error ZeroAddress(); + error SameAddress(); + + // values + error ZeroAmount(); + error ZeroRewards(); + + // pubkeys + error PubkeyExists(); + error MalformedPubkey(); + error SamePubkey(); + error EmptyPubkey(); + error PubkeyAllowedOnlyOnce(); + + // redirections + error SameRedirectionAuthorizer(); + error Redirected(); + + // validators + error ValidatorNotExists(); + error ValidatorExists(); + error ValidatorNotActive(); + error ValidatorDelegationLimitExceeded(); + error WrongValidatorStatus(); + + // requests + error RequestedCompleted(); + error RequestExists(); + error RequestNotExists(); + + // transfers + error TransfersNotAllowed(); + error TransferFailed(); + + // updater + error SFCAlreadyUpdated(); + error SFCWrongVersion(); + error SFCGovAlreadyUpdated(); + error SFCWrongGovVersion(); + + // governance + error GovVotesRecountFailed(); + + // staking + error LockedStakeGreaterThanTotalStake(); + error InsufficientSelfStake(); + error NotEnoughUnlockedStake(); + error NotEnoughLockedStake(); + error NotEnoughTimePassed(); + error NotEnoughEpochsPassed(); + error StakeIsFullySlashed(); + error IncorrectDuration(); + error ValidatorLockupTooShort(); + error TooManyReLocks(); + error TooFrequentReLocks(); + error LockupDurationDecreased(); + error AlreadyLockedUp(); + error NotLockedUp(); + + // stashing + error NothingToStash(); + + // slashing + error ValidatorNotSlashed(); + error RefundRatioTooHigh(); + event DeactivatedValidator(uint256 indexed validatorID, uint256 deactivatedEpoch, uint256 deactivatedTime); event ChangedValidatorStatus(uint256 indexed validatorID, uint256 status); @@ -19,7 +90,9 @@ contract SFCBase is SFCState { } modifier onlyDriver() { - require(isNode(msg.sender), "caller is not the NodeDriverAuth contract"); + if (!isNode(msg.sender)) { + revert NotDriverAuth(); + } _; } @@ -98,7 +171,9 @@ contract SFCBase is SFCState { abi.encodeWithSignature("recountVotes(address,address)", delegator, validatorAuth) ); // Don't revert if recountVotes failed unless strict mode enabled - require(success || !strict, "gov votes recounting failed"); + if (!success && strict) { + revert GovVotesRecountFailed(); + } } } @@ -123,7 +198,9 @@ contract SFCBase is SFCState { } function _syncValidator(uint256 validatorID, bool syncPubkey) public { - require(_validatorExists(validatorID), "validator doesn't exist"); + if (!_validatorExists(validatorID)) { + revert ValidatorNotExists(); + } // emit special log for node uint256 weight = getValidator[validatorID].receivedStake; if (getValidator[validatorID].status != OK_STATUS) { diff --git a/contracts/sfc/SFCLib.sol b/contracts/sfc/SFCLib.sol index 3d65ff0..c38fda4 100644 --- a/contracts/sfc/SFCLib.sol +++ b/contracts/sfc/SFCLib.sol @@ -78,7 +78,9 @@ contract SFCLib is SFCBase { _rewardsStash[delegator][toValidatorID].unlockedReward = rewards; _mintNativeToken(stake); if (lockedStake != 0) { - require(lockedStake <= stake, "locked stake is greater than the whole stake"); + if (lockedStake > stake) { + revert LockedStakeGreaterThanTotalStake(); + } LockedDelegation storage ld = getLockupInfo[delegator][toValidatorID]; ld.lockedStake = lockedStake; ld.fromEpoch = lockupFromEpoch; @@ -94,9 +96,15 @@ contract SFCLib is SFCBase { */ function createValidator(bytes calldata pubkey) external payable { - require(msg.value >= c.minSelfStake(), "insufficient self-stake"); - require(pubkey.length > 0, "empty pubkey"); - require(pubkeyHashToValidatorID[keccak256(pubkey)] == 0, "already used"); + if (msg.value < c.minSelfStake()) { + revert InsufficientSelfStake(); + } + if (pubkey.length == 0) { + revert EmptyPubkey(); + } + if (pubkeyHashToValidatorID[keccak256(pubkey)] != 0) { + revert PubkeyExists(); + } _createValidator(msg.sender, pubkey); _delegate(msg.sender, lastValidatorID, msg.value); } @@ -116,7 +124,9 @@ contract SFCLib is SFCBase { uint256 deactivatedEpoch, uint256 deactivatedTime ) internal { - require(getValidatorID[auth] == 0, "validator already exists"); + if (getValidatorID[auth] != 0) { + revert ValidatorExists(); + } getValidatorID[auth] = validatorID; getValidator[validatorID].status = status; getValidator[validatorID].createdEpoch = createdEpoch; @@ -151,14 +161,22 @@ contract SFCLib is SFCBase { } function _delegate(address delegator, uint256 toValidatorID, uint256 amount) internal { - require(_validatorExists(toValidatorID), "validator doesn't exist"); - require(getValidator[toValidatorID].status == OK_STATUS, "validator isn't active"); + if (!_validatorExists(toValidatorID)) { + revert ValidatorNotExists(); + } + if (getValidator[toValidatorID].status != OK_STATUS) { + revert ValidatorNotActive(); + } _rawDelegate(delegator, toValidatorID, amount, true); - require(_checkDelegatedStakeLimit(toValidatorID), "validator's delegations limit is exceeded"); + if (!_checkDelegatedStakeLimit(toValidatorID)) { + revert ValidatorDelegationLimitExceeded(); + } } function _rawDelegate(address delegator, uint256 toValidatorID, uint256 amount, bool strict) internal { - require(amount > 0, "zero amount"); + if (amount == 0) { + revert ZeroAmount(); + } _stashRewards(delegator, toValidatorID); @@ -182,7 +200,9 @@ contract SFCLib is SFCBase { (bool success, ) = voteBookAddress.call{gas: gas}( abi.encodeWithSignature("recountVotes(address,address)", delegator, validatorAuth) ); - require(success || !strict, "gov votes recounting failed"); + if (!success && strict) { + revert GovVotesRecountFailed(); + } } function _rawUndelegate( @@ -204,15 +224,14 @@ contract SFCLib is SFCBase { if (selfStakeAfterwards != 0 && getValidator[toValidatorID].status == OK_STATUS) { if (!(selfStakeAfterwards >= c.minSelfStake())) { if (forceful) { - revert("insufficient self-stake"); + revert InsufficientSelfStake(); } else { _setValidatorDeactivated(toValidatorID, WITHDRAWN_BIT); } } - require( - !checkDelegatedStake || _checkDelegatedStakeLimit(toValidatorID), - "validator's delegations limit is exceeded" - ); + if (checkDelegatedStake && !_checkDelegatedStakeLimit(toValidatorID)) { + revert ValidatorDelegationLimitExceeded(); + } } else { _setValidatorDeactivated(toValidatorID, WITHDRAWN_BIT); } @@ -225,10 +244,17 @@ contract SFCLib is SFCBase { _stashRewards(delegator, toValidatorID); - require(amount > 0, "zero amount"); - require(amount <= getUnlockedStake(delegator, toValidatorID), "not enough unlocked stake"); + if (amount == 0) { + revert ZeroAmount(); + } + + if (amount > getUnlockedStake(delegator, toValidatorID)) { + revert NotEnoughUnlockedStake(); + } - require(getWithdrawalRequest[delegator][toValidatorID][wrID].amount == 0, "wrID already exists"); + if (getWithdrawalRequest[delegator][toValidatorID][wrID].amount != 0) { + revert RequestExists(); + } _rawUndelegate(delegator, toValidatorID, amount, true, false, true); @@ -263,7 +289,9 @@ contract SFCLib is SFCBase { function _withdraw(address delegator, uint256 toValidatorID, uint256 wrID, address payable receiver) private { WithdrawalRequest memory request = getWithdrawalRequest[delegator][toValidatorID][wrID]; - require(request.epoch != 0, "request doesn't exist"); + if (request.epoch == 0) { + revert RequestNotExists(); + } uint256 requestTime = request.time; uint256 requestEpoch = request.epoch; @@ -275,8 +303,13 @@ contract SFCLib is SFCBase { requestEpoch = getValidator[toValidatorID].deactivatedEpoch; } - require(_now() >= requestTime + c.withdrawalPeriodTime(), "not enough time passed"); - require(currentEpoch() >= requestEpoch + c.withdrawalPeriodEpochs(), "not enough epochs passed"); + if (_now() < requestTime + c.withdrawalPeriodTime()) { + revert NotEnoughTimePassed(); + } + + if (currentEpoch() < requestEpoch + c.withdrawalPeriodEpochs()) { + revert NotEnoughEpochsPassed(); + } uint256 amount = getWithdrawalRequest[delegator][toValidatorID][wrID].amount; bool isCheater = isSlashed(toValidatorID); @@ -284,10 +317,14 @@ contract SFCLib is SFCBase { delete getWithdrawalRequest[delegator][toValidatorID][wrID]; totalSlashedStake += penalty; - require(amount > penalty, "stake is fully slashed"); + if (amount <= penalty) { + revert StakeIsFullySlashed(); + } // It's important that we transfer after erasing (protection against Re-Entrancy) (bool sent, ) = receiver.call{value: amount - penalty}(""); - require(sent, "Failed to send FTM"); + if (!sent) { + revert TransferFailed(); + } _burnFTM(penalty); emit Withdrawn(delegator, toValidatorID, wrID, amount); @@ -298,7 +335,9 @@ contract SFCLib is SFCBase { } function deactivateValidator(uint256 validatorID, uint256 status) external onlyDriver { - require(status != OK_STATUS, "wrong status"); + if (status == OK_STATUS) { + revert WrongValidatorStatus(); + } _setValidatorDeactivated(validatorID, status); _syncValidator(validatorID, false); @@ -397,7 +436,9 @@ contract SFCLib is SFCBase { } function stashRewards(address delegator, uint256 toValidatorID) external { - require(_stashRewards(delegator, toValidatorID), "nothing to stash"); + if (!_stashRewards(delegator, toValidatorID)) { + revert NothingToStash(); + } } function _stashRewards(address delegator, uint256 toValidatorID) internal returns (bool updated) { @@ -423,7 +464,9 @@ contract SFCLib is SFCBase { _stashRewards(delegator, toValidatorID); rewards = _rewardsStash[delegator][toValidatorID]; uint256 totalReward = rewards.unlockedReward + rewards.lockupBaseReward + rewards.lockupExtraReward; - require(totalReward != 0, "zero rewards"); + if (totalReward == 0) { + revert ZeroRewards(); + } delete _rewardsStash[delegator][toValidatorID]; // It's important that we mint after erasing (protection against Re-Entrancy) _mintNativeToken(totalReward); @@ -437,7 +480,10 @@ contract SFCLib is SFCBase { (bool sent, ) = _receiverOf(delegator).call{ value: rewards.lockupExtraReward + rewards.lockupBaseReward + rewards.unlockedReward }(""); - require(sent, "Failed to send FTM"); + + if (!sent) { + revert TransferFailed(); + } emit ClaimedRewards( delegator, @@ -500,21 +546,29 @@ contract SFCLib is SFCBase { uint256 amount, bool relock ) internal { - require(!_redirected(delegator), "redirected"); - require(amount <= getUnlockedStake(delegator, toValidatorID), "not enough stake"); - require(getValidator[toValidatorID].status == OK_STATUS, "validator isn't active"); + if (_redirected(delegator)) { + revert Redirected(); + } + + if (amount > getUnlockedStake(delegator, toValidatorID)) { + revert NotEnoughUnlockedStake(); + } + + if (getValidator[toValidatorID].status != OK_STATUS) { + revert ValidatorNotActive(); + } + + if (lockupDuration < c.minLockupDuration() || lockupDuration > c.maxLockupDuration()) { + revert IncorrectDuration(); + } - require( - lockupDuration >= c.minLockupDuration() && lockupDuration <= c.maxLockupDuration(), - "incorrect duration" - ); uint256 endTime = _now() + lockupDuration; address validatorAddr = getValidator[toValidatorID].auth; - if (delegator != validatorAddr) { - require( - getLockupInfo[validatorAddr][toValidatorID].endTime + 30 * 24 * 60 * 60 >= endTime, - "validator's lockup will end too early" - ); + if ( + delegator != validatorAddr && + getLockupInfo[validatorAddr][toValidatorID].endTime + 30 * 24 * 60 * 60 < endTime + ) { + revert ValidatorLockupTooShort(); } _stashRewards(delegator, toValidatorID); @@ -528,16 +582,21 @@ contract SFCLib is SFCBase { uint256 penalty = _popNonStashedUnlockPenalty(delegator, toValidatorID, ld.lockedStake, ld.lockedStake); if (penalty != 0) { penalties.push(Penalty(penalty, ld.endTime)); - require(penalties.length <= 30, "too many ongoing relocks"); - require( - amount > ld.lockedStake / 100 || penalties.length <= 3 || endTime >= ld.endTime + 14 * 24 * 60 * 60, - "too frequent relocks" - ); + if (penalties.length > 30) { + revert TooManyReLocks(); + } + if ( + amount <= ld.lockedStake / 100 && penalties.length > 3 && endTime < ld.endTime + 14 * 24 * 60 * 60 + ) { + revert TooFrequentReLocks(); + } } } // check lockup duration after _stashRewards, which has erased previous lockup if it has unlocked already - require(lockupDuration >= ld.duration, "lockup duration cannot decrease"); + if (lockupDuration < ld.duration) { + revert LockupDurationDecreased(); + } ld.lockedStake = ld.lockedStake + amount; ld.fromEpoch = currentEpoch(); @@ -549,14 +608,20 @@ contract SFCLib is SFCBase { function lockStake(uint256 toValidatorID, uint256 lockupDuration, uint256 amount) public { address delegator = msg.sender; - require(amount > 0, "zero amount"); - require(!isLockedUp(delegator, toValidatorID), "already locked up"); + if (amount == 0) { + revert ZeroAmount(); + } + if (isLockedUp(delegator, toValidatorID)) { + revert AlreadyLockedUp(); + } _lockStake(delegator, toValidatorID, lockupDuration, amount, false); } function relockStake(uint256 toValidatorID, uint256 lockupDuration, uint256 amount) public { address delegator = msg.sender; - require(isLockedUp(delegator, toValidatorID), "not locked up"); + if (!isLockedUp(delegator, toValidatorID)) { + revert NotLockedUp(); + } _lockStake(delegator, toValidatorID, lockupDuration, amount, true); } @@ -607,10 +672,18 @@ contract SFCLib is SFCBase { address delegator = msg.sender; LockedDelegation storage ld = getLockupInfo[delegator][toValidatorID]; - require(amount > 0, "zero amount"); - require(isLockedUp(delegator, toValidatorID), "not locked up"); - require(amount <= ld.lockedStake, "not enough locked stake"); - require(!_redirected(delegator), "redirected"); + if (amount == 0) { + revert ZeroAmount(); + } + if (!isLockedUp(delegator, toValidatorID)) { + revert NotLockedUp(); + } + if (amount > ld.lockedStake) { + revert NotEnoughLockedStake(); + } + if (_redirected(delegator)) { + revert Redirected(); + } _stashRewards(delegator, toValidatorID); @@ -622,7 +695,9 @@ contract SFCLib is SFCBase { if (penalty != 0) { _rawUndelegate(delegator, toValidatorID, penalty, true, false, false); (bool success, ) = treasuryAddress.call{value: penalty}(""); - require(success, "Failed to send penalty"); + if (!success) { + revert TransferFailed(); + } } emit UnlockedStake(delegator, toValidatorID, amount, penalty); @@ -630,8 +705,12 @@ contract SFCLib is SFCBase { } function updateSlashingRefundRatio(uint256 validatorID, uint256 refundRatio) external onlyOwner { - require(isSlashed(validatorID), "validator isn't slashed"); - require(refundRatio <= Decimal.unit(), "must be less than or equal to 1.0"); + if (!isSlashed(validatorID)) { + revert ValidatorNotSlashed(); + } + if (refundRatio > Decimal.unit()) { + revert RefundRatioTooHigh(); + } slashingRefundRatio[validatorID] = refundRatio; emit UpdatedSlashingRefundRatio(validatorID, refundRatio); } diff --git a/contracts/sfc/Updater.sol b/contracts/sfc/Updater.sol index 9b1324d..634167a 100644 --- a/contracts/sfc/Updater.sol +++ b/contracts/sfc/Updater.sol @@ -30,6 +30,12 @@ contract Updater { address public voteBook; address public owner; + error ZeroAddress(); + error SFCAlreadyUpdated(); + error SFCWrongVersion(); + error SFCGovAlreadyUpdated(); + error SFCWrongGovVersion(); + constructor( address _sfcFrom, address _sfcLib, @@ -47,20 +53,29 @@ contract Updater { voteBook = _voteBook; owner = _owner; address sfcTo = address(0xFC00FACE00000000000000000000000000000000); - require( - sfcFrom != address(0) && - sfcLib != address(0) && - sfcConsts != address(0) && - govTo != address(0) && - govFrom != address(0) && - voteBook != address(0) && - owner != address(0), - "0 address" - ); - require(Version(sfcTo).version() == "303", "SFC already updated"); - require(Version(sfcFrom).version() == "304", "wrong SFC version"); - require(GovVersion(govTo).version() == "0001", "gov already updated"); - require(GovVersion(govFrom).version() == "0002", "wrong gov version"); + if ( + sfcFrom == address(0) || + sfcLib == address(0) || + sfcConsts == address(0) || + govTo == address(0) || + govFrom == address(0) || + voteBook == address(0) || + owner == address(0) + ) { + revert ZeroAddress(); + } + if (Version(sfcTo).version() != "303") { + revert SFCAlreadyUpdated(); + } + if (Version(sfcFrom).version() != "304") { + revert SFCWrongVersion(); + } + if (GovVersion(govTo).version() != "0001") { + revert SFCGovAlreadyUpdated(); + } + if (GovVersion(govFrom).version() != "0002") { + revert SFCWrongGovVersion(); + } } function execute() external { diff --git a/test/NodeDriver.ts b/test/NodeDriver.ts index 937d2bd..d71f431 100644 --- a/test/NodeDriver.ts +++ b/test/NodeDriver.ts @@ -92,14 +92,17 @@ describe('NodeDriver', () => { const account = ethers.Wallet.createRandom(); const key = ethers.encodeBytes32String('testKey'); const value = ethers.encodeBytes32String('testValue'); - await expect(this.nodeDriver.setStorage(account, key, value)).to.be.revertedWith('caller is not the backend'); + await expect(this.nodeDriver.setStorage(account, key, value)).to.be.revertedWithCustomError( + this.nodeDriver, + 'NotBackend', + ); }); }); describe('Set backend', () => { it('Should revert when not backend', async function () { const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriver.setBackend(account)).to.be.revertedWith('caller is not the backend'); + await expect(this.nodeDriver.setBackend(account)).to.be.revertedWithCustomError(this.nodeDriver, 'NotBackend'); }); }); @@ -107,7 +110,10 @@ describe('NodeDriver', () => { it('Should revert when not backend', async function () { const account = ethers.Wallet.createRandom(); const account2 = ethers.Wallet.createRandom(); - await expect(this.nodeDriver.swapCode(account, account2)).to.be.revertedWith('caller is not the backend'); + await expect(this.nodeDriver.swapCode(account, account2)).to.be.revertedWithCustomError( + this.nodeDriver, + 'NotBackend', + ); }); }); @@ -125,35 +131,44 @@ describe('NodeDriver', () => { 0, 0, ), - ).to.be.revertedWith('not callable'); + ).to.be.revertedWithCustomError(this.nodeDriver, 'NotNode'); }); }); describe('Deactivate validator', () => { it('Should revert when not node', async function () { - await expect(this.nodeDriver.deactivateValidator(0, 1)).to.be.revertedWith('not callable'); + await expect(this.nodeDriver.deactivateValidator(0, 1)).to.be.revertedWithCustomError(this.nodeDriver, 'NotNode'); }); }); describe('Set genesis delegation', () => { it('Should revert when not node', async function () { const account = ethers.Wallet.createRandom(); - await expect(this.nodeDriver.setGenesisDelegation(account, 1, 100, 0, 0, 0, 0, 0, 1000)).to.be.revertedWith( - 'not callable', - ); + await expect( + this.nodeDriver.setGenesisDelegation(account, 1, 100, 0, 0, 0, 0, 0, 1000), + ).to.be.revertedWithCustomError(this.nodeDriver, 'NotNode'); }); }); describe('Seal epoch validators', () => { it('Should revert when not node', async function () { - await expect(this.nodeDriver.sealEpochValidators([0, 1])).to.be.revertedWith('not callable'); + await expect(this.nodeDriver.sealEpochValidators([0, 1])).to.be.revertedWithCustomError( + this.nodeDriver, + 'NotNode', + ); }); }); describe('Seal epoch', () => { it('Should revert when not node', async function () { - await expect(this.nodeDriver.sealEpoch([0, 1], [0, 1], [0, 1], [0, 1])).to.be.revertedWith('not callable'); - await expect(this.nodeDriver.sealEpochV1([0, 1], [0, 1], [0, 1], [0, 1], 0)).to.be.revertedWith('not callable'); + await expect(this.nodeDriver.sealEpoch([0, 1], [0, 1], [0, 1], [0, 1])).to.be.revertedWithCustomError( + this.nodeDriver, + 'NotNode', + ); + await expect(this.nodeDriver.sealEpochV1([0, 1], [0, 1], [0, 1], [0, 1], 0)).to.be.revertedWithCustomError( + this.nodeDriver, + 'NotNode', + ); }); }); }); diff --git a/test/SFC.ts b/test/SFC.ts index d21cc0a..557ad9f 100644 --- a/test/SFC.ts +++ b/test/SFC.ts @@ -52,7 +52,7 @@ describe('SFC', () => { to: this.sfc, value: 1, }), - ).to.revertedWith('transfers not allowed'); + ).to.revertedWithCustomError(this.sfcLib, 'TransfersNotAllowed'); }); describe('Genesis validator', () => { @@ -77,13 +77,14 @@ describe('SFC', () => { }); it('Should revert when sealEpoch not called by node', async function () { - await expect(this.sfc.sealEpoch([1], [1], [1], [1], 0)).to.be.revertedWith( - 'caller is not the NodeDriverAuth contract', + await expect(this.sfc.sealEpoch([1], [1], [1], [1], 0)).to.be.revertedWithCustomError( + this.sfcLib, + 'NotDriverAuth', ); }); it('Should revert when SealEpochValidators not called by node', async function () { - await expect(this.sfc.sealEpochValidators([1])).to.be.revertedWith('caller is not the NodeDriverAuth contract'); + await expect(this.sfc.sealEpochValidators([1])).to.be.revertedWithCustomError(this.sfcLib, 'NotDriverAuth'); }); }); @@ -178,13 +179,13 @@ describe('SFC', () => { this.sfc .connect(this.validator) .createValidator(ethers.Wallet.createRandom().publicKey, { value: ethers.parseEther('0.1') }), - ).to.be.revertedWith('insufficient self-stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'InsufficientSelfStake'); }); it('Should revert when public key is empty while creating a validator', async function () { await expect( this.sfc.connect(this.validator).createValidator('0x', { value: ethers.parseEther('0.4') }), - ).to.be.revertedWith('empty pubkey'); + ).to.be.revertedWithCustomError(this.sfcLib, 'EmptyPubkey'); }); it('Should succeed and create two validators and return id of last validator', async function () { @@ -215,7 +216,7 @@ describe('SFC', () => { it('Should revert when staking to non-existing validator', async function () { await expect( this.sfc.connect(this.secondValidator).delegate(1, { value: ethers.parseEther('0.1') }), - ).to.be.revertedWith("validator doesn't exist"); + ).to.be.revertedWithCustomError(this.sfcLib, 'ValidatorNotExists'); }); it('Should succeed and stake with different delegators', async function () { @@ -330,13 +331,14 @@ describe('SFC', () => { 0, 0, ), - ).to.be.revertedWith('caller is not the NodeDriverAuth contract'); + ).to.be.revertedWithCustomError(this.sfcLib, 'NotDriverAuth'); }); it('Should revert when setGenesisDelegation is not called not node', async function () { const delegator = ethers.Wallet.createRandom(); - await expect(this.sfc.setGenesisDelegation(delegator, 1, 100, 0, 0, 0, 0, 0, 1000)).to.be.revertedWith( - 'caller is not the NodeDriverAuth contract', + await expect(this.sfc.setGenesisDelegation(delegator, 1, 100, 0, 0, 0, 0, 0, 1000)).to.be.revertedWithCustomError( + this.sfcLib, + 'NotDriverAuth', ); }); }); @@ -441,7 +443,7 @@ describe('SFC', () => { this.sfc .connect(validator) .createValidator(ethers.Wallet.createRandom().publicKey, { value: ethers.parseEther('0.1') }), - ).to.be.revertedWith('insufficient self-stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'InsufficientSelfStake'); await node.handleTx( await this.sfc.connect(validator).createValidator(pubkey, { value: ethers.parseEther('0.3175') }), @@ -451,7 +453,7 @@ describe('SFC', () => { this.sfc .connect(validator) .createValidator(ethers.Wallet.createRandom().publicKey, { value: ethers.parseEther('0.5') }), - ).to.be.revertedWith('validator already exists'); + ).to.be.revertedWithCustomError(this.sfcLib, 'ValidatorExists'); await node.handleTx( await this.sfc.connect(secondValidator).createValidator(secondPubkey, { value: ethers.parseEther('0.5') }), @@ -768,8 +770,9 @@ describe('SFC', () => { it('Should revert when deactivating validator if not Node', async function () { await this.sfc.disableNonNodeCalls(); - await expect(this.sfc.deactivateValidator(this.validatorId, 0)).to.be.revertedWith( - 'caller is not the NodeDriverAuth contract', + await expect(this.sfc.deactivateValidator(this.validatorId, 0)).to.be.revertedWithCustomError( + this.sfcLib, + 'NotDriverAuth', ); }); @@ -990,23 +993,27 @@ describe('SFC', () => { 0, 0, ), - ).to.be.revertedWith('caller is not the NodeDriver contract'); + ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'NotDriver'); }); it('Should revert when calling setGenesisDelegation if not NodeDriver', async function () { await expect( this.nodeDriverAuth.setGenesisDelegation(this.delegator, 1, 100, 0, 0, 0, 0, 0, 1000), - ).to.be.revertedWith('caller is not the NodeDriver contract'); + ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'NotDriver'); }); it('Should revert when calling deactivateValidator if not NodeDriver', async function () { - await expect(this.nodeDriverAuth.deactivateValidator(1, 0)).to.be.revertedWith( - 'caller is not the NodeDriver contract', + await expect(this.nodeDriverAuth.deactivateValidator(1, 0)).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'NotDriver', ); }); it('Should revert when calling deactivateValidator with wrong status', async function () { - await expect(this.sfc.deactivateValidator(1, 0)).to.be.revertedWith('wrong status'); + await expect(this.sfc.deactivateValidator(1, 0)).to.be.revertedWithCustomError( + this.sfcLib, + 'WrongValidatorStatus', + ); }); it('Should succeed and deactivate validator', async function () { @@ -1014,8 +1021,9 @@ describe('SFC', () => { }); it('Should revert when calling sealEpoch if not NodeDriver', async function () { - await expect(this.nodeDriverAuth.sealEpochValidators([1])).to.be.revertedWith( - 'caller is not the NodeDriver contract', + await expect(this.nodeDriverAuth.sealEpochValidators([1])).to.be.revertedWithCustomError( + this.nodeDriverAuth, + 'NotDriver', ); }); @@ -1048,7 +1056,7 @@ describe('SFC', () => { await this.sfc.advanceTime(24 * 60 * 60); await expect( this.nodeDriverAuth.sealEpoch(offlineTimes, offlineBlocks, uptimes, originatedTxsFees, 0), - ).to.be.revertedWith('caller is not the NodeDriver contract'); + ).to.be.revertedWithCustomError(this.nodeDriverAuth, 'NotDriver'); }); }); @@ -1091,11 +1099,11 @@ describe('SFC', () => { describe('Epoch getters', () => { it('Should revert when trying to unlock stake if not lockedup', async function () { - await expect(this.sfc.unlockStake(1, 10)).to.be.revertedWith('not locked up'); + await expect(this.sfc.unlockStake(1, 10)).to.be.revertedWithCustomError(this.sfcLib, 'NotLockedUp'); }); it('Should revert when trying to unlock stake if amount is 0', async function () { - await expect(this.sfc.unlockStake(1, 0)).to.be.revertedWith('zero amount'); + await expect(this.sfc.unlockStake(1, 0)).to.be.revertedWithCustomError(this.sfcLib, 'ZeroAmount'); }); it('Should succeed and return slashed status', async function () { @@ -1103,12 +1111,13 @@ describe('SFC', () => { }); it('Should revert when delegating to an unexisting validator', async function () { - await expect(this.sfc.delegate(4)).to.be.revertedWith("validator doesn't exist"); + await expect(this.sfc.delegate(4)).to.be.revertedWithCustomError(this.sfcLib, 'ValidatorNotExists'); }); it('Should revert when delegating to an unexisting validator (2)', async function () { - await expect(this.sfc.delegate(4, { value: ethers.parseEther('1') })).to.be.revertedWith( - "validator doesn't exist", + await expect(this.sfc.delegate(4, { value: ethers.parseEther('1') })).to.be.revertedWithCustomError( + this.sfcLib, + 'ValidatorNotExists', ); }); }); @@ -1207,25 +1216,34 @@ describe('SFC', () => { }); it('Should revert when withdrawing nonexistent request', async function () { - await expect(this.sfc.withdraw(this.validatorId, 0)).to.be.revertedWith("request doesn't exist"); + await expect(this.sfc.withdraw(this.validatorId, 0)).to.be.revertedWithCustomError( + this.sfcLib, + 'RequestNotExists', + ); }); it('Should revert when undelegating 0 amount', async function () { await this.blockchainNode.sealEpoch(1_000); - await expect(this.sfc.undelegate(this.validatorId, 0, 0)).to.be.revertedWith('zero amount'); + await expect(this.sfc.undelegate(this.validatorId, 0, 0)).to.be.revertedWithCustomError( + this.sfcLib, + 'ZeroAmount', + ); }); it('Should revert when undelegating if not enough unlocked stake', async function () { await this.blockchainNode.sealEpoch(1_000); - await expect(this.sfc.undelegate(this.validatorId, 0, 10)).to.be.revertedWith('not enough unlocked stake'); + await expect(this.sfc.undelegate(this.validatorId, 0, 10)).to.be.revertedWithCustomError( + this.sfcLib, + 'NotEnoughUnlockedStake', + ); }); it('Should revert when unlocking if not enough unlocked stake', async function () { await this.blockchainNode.sealEpoch(1_000); await this.sfc.connect(this.thirdDelegator).delegate(this.validatorId, { value: ethers.parseEther('1') }); - await expect(this.sfc.connect(this.thirdDelegator).unlockStake(this.validatorId, 10)).to.be.revertedWith( - 'not locked up', - ); + await expect( + this.sfc.connect(this.thirdDelegator).unlockStake(this.validatorId, 10), + ).to.be.revertedWithCustomError(this.sfcLib, 'NotLockedUp'); }); it('Should succeed and return the unlocked stake', async function () { @@ -1239,9 +1257,9 @@ describe('SFC', () => { await this.blockchainNode.sealEpoch(1_000); await this.sfc.connect(this.thirdDelegator).delegate(this.thirdValidatorId, { value: ethers.parseEther('10') }); await this.blockchainNode.sealEpoch(1_000); - await expect(this.sfc.connect(this.thirdDelegator).claimRewards(this.validatorId)).to.be.revertedWith( - 'zero rewards', - ); + await expect( + this.sfc.connect(this.thirdDelegator).claimRewards(this.validatorId), + ).to.be.revertedWithCustomError(this.sfcLib, 'ZeroRewards'); }); }); @@ -1259,7 +1277,7 @@ describe('SFC', () => { this.sfc .connect(this.thirdDelegator) .lockStake(this.validatorId, 2 * 60 * 60 * 24 * 365, ethers.parseEther('0')), - ).to.be.revertedWith('zero amount'); + ).to.be.revertedWithCustomError(this.sfcLib, 'ZeroAmount'); }); it('Should revert when locking for more than a year', async function () { @@ -1269,7 +1287,7 @@ describe('SFC', () => { this.sfc .connect(this.thirdDelegator) .lockStake(this.thirdValidatorId, 2 * 60 * 60 * 24 * 365, ethers.parseEther('1')), - ).to.be.revertedWith('incorrect duration'); + ).to.be.revertedWithCustomError(this.sfcLib, 'IncorrectDuration'); }); it('Should revert when locking for more than a validator lockup period', async function () { @@ -1279,7 +1297,7 @@ describe('SFC', () => { this.sfc .connect(this.thirdDelegator) .lockStake(this.thirdValidatorId, 60 * 60 * 24 * 364, ethers.parseEther('1')), - ).to.be.revertedWith("validator's lockup will end too early"); + ).to.be.revertedWithCustomError(this.sfcLib, 'ValidatorLockupTooShort'); await this.sfc .connect(this.thirdDelegator) .lockStake(this.thirdValidatorId, 60 * 60 * 24 * 363, ethers.parseEther('1')); @@ -1301,9 +1319,9 @@ describe('SFC', () => { .connect(this.thirdDelegator) .lockStake(this.thirdValidatorId, 60 * 60 * 24 * 14, ethers.parseEther('1')); await this.blockchainNode.sealEpoch(60 * 60 * 24 * 14); - await expect(this.sfc.unlockStake(this.thirdValidatorId, ethers.parseEther('10'))).to.be.revertedWith( - 'not locked up', - ); + await expect( + this.sfc.unlockStake(this.thirdValidatorId, ethers.parseEther('10')), + ).to.be.revertedWithCustomError(this.sfcLib, 'NotLockedUp'); }); it('Should revert when unlocking more than locked stake', async function () { @@ -1315,7 +1333,7 @@ describe('SFC', () => { await this.blockchainNode.sealEpoch(60 * 60 * 24 * 14); await expect( this.sfc.connect(this.thirdDelegator).unlockStake(this.thirdValidatorId, ethers.parseEther('10')), - ).to.be.revertedWith('not enough locked stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'NotEnoughLockedStake'); }); it('Should succeed and scale unlocking penalty', async function () { @@ -1346,7 +1364,7 @@ describe('SFC', () => { await expect( this.sfc.connect(this.thirdDelegator).unlockStake(this.thirdValidatorId, ethers.parseEther('0.51')), - ).to.be.revertedWith('not enough locked stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'NotEnoughLockedStake'); expect( await this.sfc .connect(this.thirdDelegator) @@ -1387,7 +1405,7 @@ describe('SFC', () => { await expect( this.sfc.connect(this.thirdDelegator).unlockStake(this.thirdValidatorId, ethers.parseEther('0.51')), - ).to.be.revertedWith('not enough locked stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'NotEnoughLockedStake'); expect( await this.sfc .connect(this.thirdDelegator) @@ -1405,7 +1423,7 @@ describe('SFC', () => { await expect( this.sfc.connect(this.thirdDelegator).unlockStake(this.thirdValidatorId, ethers.parseEther('1.51')), - ).to.be.revertedWith('not enough locked stake'); + ).to.be.revertedWithCustomError(this.sfcLib, 'NotEnoughLockedStake'); expect( await this.sfc .connect(this.thirdDelegator) @@ -1467,13 +1485,16 @@ describe('SFC', () => { await expect( this.sfc.connect(this.validator).updateSlashingRefundRatio(this.thirdValidatorId, 1), - ).to.be.revertedWith("validator isn't slashed"); + ).to.be.revertedWithCustomError(this.sfcLib, 'ValidatorNotSlashed'); await this.blockchainNode.sealEpoch(60 * 60 * 24 * 14); }); it('Should revert when syncing if validator does not exist', async function () { - await expect(this.sfc._syncValidator(33, false)).to.be.revertedWith("validator doesn't exist"); + await expect(this.sfc._syncValidator(33, false)).to.be.revertedWithCustomError( + this.sfcLib, + 'ValidatorNotExists', + ); }); }); }); @@ -1676,7 +1697,7 @@ describe('SFC', () => { await this.blockchainNode.sealEpoch(60 * 60 * 24); await expect( this.sfc.connect(this.firstDelegator).relockStake(this.validatorId, 60 * 60 * 24 * 20, ethers.parseEther('0')), - ).to.be.revertedWith('too frequent relocks'); + ).to.be.revertedWithCustomError(this.sfcLib, 'TooFrequentReLocks'); // 4 await this.sfc.advanceTime(60 * 60 * 24 * 14); @@ -1686,7 +1707,7 @@ describe('SFC', () => { await this.blockchainNode.sealEpoch(60 * 60 * 24); await expect( this.sfc.connect(this.firstDelegator).relockStake(this.validatorId, 60 * 60 * 24 * 20, ethers.parseEther('0')), - ).to.be.revertedWith('too frequent relocks'); + ).to.be.revertedWithCustomError(this.sfcLib, 'TooFrequentReLocks'); for (let i = 5; i <= 40; i++) { // 5-40