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