From 93ba4f69513295bab85fb1b8047b583d0b2b4a1c Mon Sep 17 00:00:00 2001 From: Mike-CZ Date: Tue, 22 Oct 2024 11:51:43 +0200 Subject: [PATCH 1/6] 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/6] 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/6] 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/6] 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/6] 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/6] 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;