diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0b15ea1f9..976509a7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -27,7 +27,7 @@ jobs: run: yarn static-check - name: Run size check - run: yarn contract-size --checkMaxSize + run: yarn contract-size - name: Run tests run: yarn test diff --git a/.gitignore b/.gitignore index 33ed8d41c..88e8a285e 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,8 @@ ganache-blockchain-log.txt .coverage_contracts @types/generated/ config.js +blacklist.*.js +!blacklist.test.js out/ cache/ diff --git a/.vscode/settings.json b/.vscode/settings.json index 2b8e378d4..6a00adaac 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -27,5 +27,6 @@ "licenser.license": "Custom", "licenser.customHeader": "SPDX-License-Identifier: MIT\n\nCopyright (c) 2018-@YEAR@ CENTRE SECZ\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\nSOFTWARE.\n", "licenser.useSingleLineStyle": false, - "files.autoSave": "onFocusChange" + "files.autoSave": "onFocusChange", + "solidity.formatter": "none" } diff --git a/README.md b/README.md index 5ec2cbff5..7fdcb27ae 100644 --- a/README.md +++ b/README.md @@ -82,11 +82,11 @@ To run tests and generate test coverage, run: $ yarn coverage ``` -To check the size of contracts in the repo, run the following command. Add the -flag if you want it to return an error code if a contract is larger than 24KiB: +To check the size of contracts in the repo, run the following command. -``` -$ yarn contract-size [--checkMaxSize] +```sh +$ yarn contract-size # Ignores tests +$ yarn contract-size:all # Includes all contracts ``` ## Deployment @@ -97,6 +97,11 @@ addresses of proxy admin, owner, master minter, blacklister, and pauser in `config.js`. This file must not be checked into the repository. To prevent accidental check-ins, `config.js` is in `.gitignore`. +Create a copy of the file `blacklist.test.js`, and name it +`blacklist.remote.js`. Fill in `blacklist.remote.js` with the list addresses to +blacklist. This file must not be checked into the repository. To prevent +accidental check-ins, `blacklist.remote.js` is in `.gitignore`. + Run `yarn migrate --network NETWORK`, where NETWORK is either `mainnet` or `ropsten`. diff --git a/blacklist.test.js b/blacklist.test.js new file mode 100644 index 000000000..5857030f8 --- /dev/null +++ b/blacklist.test.js @@ -0,0 +1,37 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +/** + * These are addresses used purely for tests. Deployers should + * create a `blacklist.remote.js` file and fill those in with + * correct addresses to blacklist before proceeding with the deploy. + */ +module.exports = [ + "0x04dba1194ee10112fe6c3207c0687def0e78bacf", + "0x08a8a2436fc920e6c73c3a9e9a00b8d937812ee0", + "0xb6f5ec1a0a9cd1526536d3f0426c429529471f40", + "0xbf4f36efa3ac655a1d86f6c32b648a90271443f4", + "0xf8a9ab377ce63592583767b34602e130e38ebdca", + "0xfc672c73ca5c7234edc82552e4a0c8fc247d32ac", +]; diff --git a/contracts/test/MockFiatTokenWithEditableBalanceAndBlacklistStates.sol b/contracts/test/MockFiatTokenWithEditableBalanceAndBlacklistStates.sol new file mode 100644 index 000000000..417026297 --- /dev/null +++ b/contracts/test/MockFiatTokenWithEditableBalanceAndBlacklistStates.sol @@ -0,0 +1,94 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +pragma solidity 0.6.12; + +import { FiatTokenV2_2 } from "../v2/FiatTokenV2_2.sol"; + +// solhint-disable func-name-mixedcase + +/** + * @title MockFiatTokenWithEditableBalanceAndBlacklistStates + * @dev A mock class that allows the internal balanceAndBlacklistStates to be manipulated in tests. + */ +contract MockFiatTokenWithEditableBalanceAndBlacklistStates is FiatTokenV2_2 { + /** + * @dev Allows the balanceAndBlacklistStates to be manipulated. This + * enables us to properly test the ERC20 functionalities. + */ + function setBalanceAndBlacklistStates(address _account, uint256 _state) + external + { + balanceAndBlacklistStates[_account] = _state; + } + + /** + * @dev Allows the balanceAndBlacklistStates to be read as plain values. + */ + function getBalanceAndBlacklistStates(address _account) + external + view + returns (uint256) + { + return balanceAndBlacklistStates[_account]; + } + + /** + * @dev Exposes the internal function for unit testing. + */ + function internal_setBlacklistState(address _account, bool _shouldBlacklist) + external + { + _setBlacklistState(_account, _shouldBlacklist); + } + + /** + * @dev Exposes the internal function for unit testing. + */ + function internal_setBalance(address _account, uint256 _balance) external { + _setBalance(_account, _balance); + } + + /** + * @dev Exposes the internal function for unit testing. + */ + function internal_isBlacklisted(address _account) + external + view + returns (bool) + { + return _isBlacklisted(_account); + } + + /** + * @dev Exposes the internal function for unit testing. + */ + function internal_balanceOf(address _account) + external + view + returns (uint256) + { + return _balanceOf(_account); + } +} diff --git a/contracts/test/UpgradedFiatTokenV2_2.sol b/contracts/test/UpgradedFiatTokenV2_2.sol new file mode 100644 index 000000000..0a318b9e2 --- /dev/null +++ b/contracts/test/UpgradedFiatTokenV2_2.sol @@ -0,0 +1,35 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +pragma solidity 0.6.12; + +import { FiatTokenV2_2 } from "../v2/FiatTokenV2_2.sol"; + +/** + * @title UpgradedFiatTokenV2_2 + * @dev ERC20 Token backed by fiat reserves + */ +contract UpgradedFiatTokenV2_2 is FiatTokenV2_2 { + +} diff --git a/contracts/test/UpgradedFiatTokenV2_2NewFieldsTest.sol b/contracts/test/UpgradedFiatTokenV2_2NewFieldsTest.sol new file mode 100644 index 000000000..605ba94ef --- /dev/null +++ b/contracts/test/UpgradedFiatTokenV2_2NewFieldsTest.sol @@ -0,0 +1,76 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +pragma solidity 0.6.12; + +import { FiatTokenV2_2 } from "../v2/FiatTokenV2_2.sol"; + +/** + * @title UpgradedFiatTokenV2_2NewFieldsTest + * @dev ERC20 Token backed by fiat reserves + */ +contract UpgradedFiatTokenV2_2NewFieldsTest is FiatTokenV2_2 { + bool public newBool; + address public newAddress; + uint256 public newUint; + bool internal initializedV2; + + function initialize( + string calldata tokenName, + string calldata tokenSymbol, + string calldata tokenCurrency, + uint8 tokenDecimals, + address newMasterMinter, + address newPauser, + address newBlacklister, + address newOwner, + bool _newBool, + address _newAddress, + uint256 _newUint + ) external { + super.initialize( + tokenName, + tokenSymbol, + tokenCurrency, + tokenDecimals, + newMasterMinter, + newPauser, + newBlacklister, + newOwner + ); + initV2(_newBool, _newAddress, _newUint); + } + + function initV2( + bool _newBool, + address _newAddress, + uint256 _newUint + ) public { + require(!initializedV2, "contract is already initialized"); + newBool = _newBool; + newAddress = _newAddress; + newUint = _newUint; + initializedV2 = true; + } +} diff --git a/contracts/v1/Blacklistable.sol b/contracts/v1/Blacklistable.sol index 8bf481532..0db7c8165 100644 --- a/contracts/v1/Blacklistable.sol +++ b/contracts/v1/Blacklistable.sol @@ -30,7 +30,7 @@ import { Ownable } from "./Ownable.sol"; * @title Blacklistable Token * @dev Allows accounts to be blacklisted by a "blacklister" role */ -contract Blacklistable is Ownable { +abstract contract Blacklistable is Ownable { address public blacklister; mapping(address => bool) internal _deprecatedBlacklisted; @@ -55,7 +55,7 @@ contract Blacklistable is Ownable { */ modifier notBlacklisted(address _account) { require( - !_deprecatedBlacklisted[_account], + !_isBlacklisted(_account), "Blacklistable: account is blacklisted" ); _; @@ -66,7 +66,7 @@ contract Blacklistable is Ownable { * @param _account The address to check */ function isBlacklisted(address _account) external view returns (bool) { - return _deprecatedBlacklisted[_account]; + return _isBlacklisted(_account); } /** @@ -74,7 +74,7 @@ contract Blacklistable is Ownable { * @param _account The address to blacklist */ function blacklist(address _account) external onlyBlacklister { - _deprecatedBlacklisted[_account] = true; + _blacklist(_account); emit Blacklisted(_account); } @@ -83,7 +83,7 @@ contract Blacklistable is Ownable { * @param _account The address to remove from the blacklist */ function unBlacklist(address _account) external onlyBlacklister { - _deprecatedBlacklisted[_account] = false; + _unBlacklist(_account); emit UnBlacklisted(_account); } @@ -95,4 +95,27 @@ contract Blacklistable is Ownable { blacklister = _newBlacklister; emit BlacklisterChanged(blacklister); } + + /** + * @dev Checks if account is blacklisted. + * @param _account The address to check. + * @return true if the account is blacklisted, false otherwise. + */ + function _isBlacklisted(address _account) + internal + virtual + view + returns (bool); + + /** + * @dev Helper method that blacklists an account. + * @param _account The address to blacklist. + */ + function _blacklist(address _account) internal virtual; + + /** + * @dev Helper method that unblacklists an account. + * @param _account The address to unblacklist. + */ + function _unBlacklist(address _account) internal virtual; } diff --git a/contracts/v1/FiatTokenV1.sol b/contracts/v1/FiatTokenV1.sol index 2c911982f..4e52f1eef 100644 --- a/contracts/v1/FiatTokenV1.sol +++ b/contracts/v1/FiatTokenV1.sol @@ -44,6 +44,9 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { address public masterMinter; bool internal initialized; + /// @dev A mapping that stores the balance and blacklist states for a given address. + /// The first bit defines whether the address is blacklisted (1 if blacklisted, 0 otherwise). + /// The last 255 bits define the balance for the address. mapping(address => uint256) internal balanceAndBlacklistStates; mapping(address => mapping(address => uint256)) internal allowed; uint256 internal totalSupply_ = 0; @@ -128,9 +131,7 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { ); totalSupply_ = totalSupply_.add(_amount); - balanceAndBlacklistStates[_to] = balanceAndBlacklistStates[_to].add( - _amount - ); + _setBalance(_to, _balanceOf(_to).add(_amount)); minterAllowed[msg.sender] = mintingAllowedAmount.sub(_amount); emit Mint(msg.sender, _to, _amount); emit Transfer(address(0), _to, _amount); @@ -190,6 +191,7 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { /** * @dev Get token balance of an account * @param account address The account + * @return balance The fiat token balance of the account */ function balanceOf(address account) external @@ -197,7 +199,7 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { view returns (uint256) { - return balanceAndBlacklistStates[account]; + return _balanceOf(account); } /** @@ -297,16 +299,12 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { require(from != address(0), "ERC20: transfer from the zero address"); require(to != address(0), "ERC20: transfer to the zero address"); require( - value <= balanceAndBlacklistStates[from], + value <= _balanceOf(from), "ERC20: transfer amount exceeds balance" ); - balanceAndBlacklistStates[from] = balanceAndBlacklistStates[from].sub( - value - ); - balanceAndBlacklistStates[to] = balanceAndBlacklistStates[to].add( - value - ); + _setBalance(from, _balanceOf(from).sub(value)); + _setBalance(to, _balanceOf(to).add(value)); emit Transfer(from, to, value); } @@ -356,12 +354,12 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { onlyMinters notBlacklisted(msg.sender) { - uint256 balance = balanceAndBlacklistStates[msg.sender]; + uint256 balance = _balanceOf(msg.sender); require(_amount > 0, "FiatToken: burn amount not greater than 0"); require(balance >= _amount, "FiatToken: burn amount exceeds balance"); totalSupply_ = totalSupply_.sub(_amount); - balanceAndBlacklistStates[msg.sender] = balance.sub(_amount); + _setBalance(msg.sender, balance.sub(_amount)); emit Burn(msg.sender, _amount); emit Transfer(msg.sender, address(0), _amount); } @@ -374,4 +372,66 @@ contract FiatTokenV1 is AbstractFiatTokenV1, Ownable, Pausable, Blacklistable { masterMinter = _newMasterMinter; emit MasterMinterChanged(masterMinter); } + + /** + * @inheritdoc Blacklistable + */ + function _blacklist(address _account) internal override { + _setBlacklistState(_account, true); + } + + /** + * @inheritdoc Blacklistable + */ + function _unBlacklist(address _account) internal override { + _setBlacklistState(_account, false); + } + + /** + * @dev Helper method that sets the blacklist state of an account. + * @param _account The address of the account. + * @param _shouldBlacklist True if the account should be blacklisted, false if the account should be unblacklisted. + */ + function _setBlacklistState(address _account, bool _shouldBlacklist) + internal + virtual + { + _deprecatedBlacklisted[_account] = _shouldBlacklist; + } + + /** + * @dev Helper method that sets the balance of an account. + * @param _account The address of the account. + * @param _balance The new fiat token balance of the account. + */ + function _setBalance(address _account, uint256 _balance) internal virtual { + balanceAndBlacklistStates[_account] = _balance; + } + + /** + * @inheritdoc Blacklistable + */ + function _isBlacklisted(address _account) + internal + virtual + override + view + returns (bool) + { + return _deprecatedBlacklisted[_account]; + } + + /** + * @dev Helper method to obtain the balance of an account. + * @param _account The address of the account. + * @return The fiat token balance of the account. + */ + function _balanceOf(address _account) + internal + virtual + view + returns (uint256) + { + return balanceAndBlacklistStates[_account]; + } } diff --git a/contracts/v2/FiatTokenV2_1.sol b/contracts/v2/FiatTokenV2_1.sol index 0d182f440..3fb02da2e 100644 --- a/contracts/v2/FiatTokenV2_1.sol +++ b/contracts/v2/FiatTokenV2_1.sol @@ -41,11 +41,11 @@ contract FiatTokenV2_1 is FiatTokenV2 { // solhint-disable-next-line reason-string require(_initializedVersion == 1); - uint256 lockedAmount = balanceAndBlacklistStates[address(this)]; + uint256 lockedAmount = _balanceOf(address(this)); if (lockedAmount > 0) { _transfer(address(this), lostAndFound, lockedAmount); } - _deprecatedBlacklisted[address(this)] = true; + _blacklist(address(this)); _initializedVersion = 2; } diff --git a/contracts/v2/FiatTokenV2_2.sol b/contracts/v2/FiatTokenV2_2.sol index be7b5444a..4fd9d2a5a 100644 --- a/contracts/v2/FiatTokenV2_2.sol +++ b/contracts/v2/FiatTokenV2_2.sol @@ -24,6 +24,7 @@ pragma solidity 0.6.12; +import { Blacklistable } from "../v1/Blacklistable.sol"; import { FiatTokenV2_1 } from "./FiatTokenV2_1.sol"; import { EIP712 } from "../util/EIP712.sol"; @@ -37,11 +38,22 @@ contract FiatTokenV2_2 is FiatTokenV2_1 { /** * @notice Initialize v2.2 */ - function initializeV2_2() external { + function initializeV2_2(address[] calldata accountsToBlacklist) external { // solhint-disable-next-line reason-string require(_initializedVersion == 2); - _CACHED_CHAIN_ID = _chainId(); + // Add previously blacklisted accounts to the new blacklist data structure + // and remove them from the old blacklist data structure. + for (uint256 i = 0; i < accountsToBlacklist.length; i++) { + require( + _deprecatedBlacklisted[accountsToBlacklist[i]], + "FiatTokenV2_2: Blacklisting previously unblacklisted account!" + ); + _blacklist(accountsToBlacklist[i]); + delete _deprecatedBlacklisted[accountsToBlacklist[i]]; + } + _blacklist(address(this)); + delete _deprecatedBlacklisted[address(this)]; _initializedVersion = 3; } @@ -156,4 +168,75 @@ contract FiatTokenV2_2 is FiatTokenV2_1 { ) external whenNotPaused { _cancelAuthorization(authorizer, nonce, signature); } + + /** + * @dev Helper method that sets the blacklist state of an account on balanceAndBlacklistStates. + * If _shouldBlacklist is true, we apply a (1 << 255) bitmask with an OR operation on the + * account's balanceAndBlacklistState. This flips the high bit for the account to 1, + * indicating that the account is blacklisted. + * + * If _shouldBlacklist if false, we reset the account's balanceAndBlacklistStates to their + * balances. This clears the high bit for the account, indicating that the account is unblacklisted. + * @param _account The address of the account. + * @param _shouldBlacklist True if the account should be blacklisted, false if the account should be unblacklisted. + */ + function _setBlacklistState(address _account, bool _shouldBlacklist) + internal + override + { + balanceAndBlacklistStates[_account] = _shouldBlacklist + ? balanceAndBlacklistStates[_account] | (1 << 255) + : _balanceOf(_account); + } + + /** + * @dev Helper method that sets the balance of an account on balanceAndBlacklistStates. + * Since balances are stored in the last 255 bits of the balanceAndBlacklistStates value, + * we need to ensure that the updated balance does not exceed (2^255 - 1). + * Since blacklisted accounts' balances cannot be updated, the method will also + * revert if the account is blacklisted + * @param _account The address of the account. + * @param _balance The new fiat token balance of the account (max: (2^255 - 1)). + */ + function _setBalance(address _account, uint256 _balance) internal override { + require( + _balance <= ((1 << 255) - 1), + "FiatTokenV2_2: Balance exceeds (2^255 - 1)" + ); + require( + !_isBlacklisted(_account), + "FiatTokenV2_2: Account is blacklisted" + ); + + balanceAndBlacklistStates[_account] = _balance; + } + + /** + * @inheritdoc Blacklistable + */ + function _isBlacklisted(address _account) + internal + override + view + returns (bool) + { + return balanceAndBlacklistStates[_account] >> 255 == 1; + } + + /** + * @dev Helper method to obtain the balance of an account. Since balances + * are stored in the last 255 bits of the balanceAndBlacklistStates value, + * we apply a ((1 << 255) - 1) bit bitmask with an AND operation on the + * balanceAndBlacklistState to obtain the balance. + * @param _account The address of the account. + * @return The fiat token balance of the account. + */ + function _balanceOf(address _account) + internal + override + view + returns (uint256) + { + return balanceAndBlacklistStates[_account] & ((1 << 255) - 1); + } } diff --git a/contracts/v2/upgrader/V2_2Upgrader.sol b/contracts/v2/upgrader/V2_2Upgrader.sol index e00eb1e90..14382b773 100644 --- a/contracts/v2/upgrader/V2_2Upgrader.sol +++ b/contracts/v2/upgrader/V2_2Upgrader.sol @@ -59,18 +59,31 @@ contract V2_2Upgrader is AbstractV2Upgrader { uint256 totalSupply; } + address[] private _accountsToBlacklist; + /** * @notice Constructor - * @param proxy FiatTokenProxy contract - * @param implementation FiatTokenV2_2 implementation contract - * @param newProxyAdmin Grantee of proxy admin role after upgrade + * @param proxy FiatTokenProxy contract + * @param implementation FiatTokenV2_2 implementation contract + * @param newProxyAdmin Grantee of proxy admin role after upgrade + * @param accountsToBlacklist Accounts to add to the new blacklist data structure */ constructor( FiatTokenProxy proxy, FiatTokenV2_2 implementation, - address newProxyAdmin + address newProxyAdmin, + address[] memory accountsToBlacklist ) public AbstractV2Upgrader(proxy, address(implementation), newProxyAdmin) { _helper = new V2_2UpgraderHelper(address(proxy)); + _accountsToBlacklist = accountsToBlacklist; + } + + /** + * @notice The list of blacklisted accounts to migrate to the blacklist data structure. + * @return Address[] the list of accounts to blacklist. + */ + function accountsToBlacklist() external view returns (address[] memory) { + return _accountsToBlacklist; } /** @@ -115,7 +128,7 @@ contract V2_2Upgrader is AbstractV2Upgrader { // Initialize V2 contract FiatTokenV2_2 v2_2 = FiatTokenV2_2(address(_proxy)); - v2_2.initializeV2_2(); + v2_2.initializeV2_2(_accountsToBlacklist); // Sanity test // Check metadata diff --git a/doc/v2.2_upgrade.md b/doc/v2.2_upgrade.md index 6a7c234fa..6731c42fc 100644 --- a/doc/v2.2_upgrade.md +++ b/doc/v2.2_upgrade.md @@ -28,7 +28,10 @@ $ MIGRATION_END= ``` -4. Run Truffle migrations using the Deployer Key, and get the address of the +4. Ensure that the `blacklist.remote.js` file in the project root folder is + configured with the correct list of addresses to blacklist. + +5. Run Truffle migrations using the Deployer Key, and get the address of the newly deployed `V2_2Upgrader` contract. ```sh @@ -41,24 +44,32 @@ >>>>>>> Deployed V2_2Upgrader at 0x12345678 <<<<<<< ``` -5. Verify that the upgrader contract is deployed correctly. Verify that the +6. Verify that the upgrader contract is deployed correctly. Verify that the values returned by `proxy()`, `implementation()` and `newProxyAdmin()` on the - `V2_2Upgrader` contract are correct. + `V2_2Upgrader` contract are correct using a block explorer. -6. Verify that the `V2_2UpgraderHelper` contract is deployed correctly. Retrieve +7. Verify that the `V2_2UpgraderHelper` contract is deployed correctly. Retrieve the address from the `V2_2Upgrader` contract `helper()` method, and verify that the return values of each view methods correspond with the `FiatTokenProxy` contract to be upgraded. -7. Using the Admin Key, transfer the proxy admin role to the `V2_2Upgrader` +8. Verify that the list of accounts to blacklist is correct, and is set + correctly in the upgrader contract. Run the following command to check the + list. + + ```sh + $ yarn truffle exec scripts/validateAccountsToBlacklist.js ${FiatTokenProxy address} ${V2_2Upgrader address} --network=${NETWORK} + ``` + +9. Using the Admin Key, transfer the proxy admin role to the `V2_2Upgrader` contract address by calling `changeAdmin(address)` method on the `FiatTokenProxy` contract. -8. Send 0.20 FiatToken (eg USDC) to the `V2_2Upgrader` contract address. - (200,000 tokens) +10. Send 0.20 FiatToken (eg USDC) to the `V2_2Upgrader` contract address. + (200,000 tokens) -9. Using the Deployer Key, call `upgrade()` (`0xd55ec697`) method on the - `V2_2Upgrader`. +11. Using the Deployer Key, call `upgrade()` (`0xd55ec697`) method on the + `V2_2Upgrader`. #### IF THE UPGRADE TRANSACTION SUCCEEDS diff --git a/migrations/7_deploy_v2_2.js b/migrations/7_deploy_v2_2.js index 1fd310fd2..ffc6d933b 100644 --- a/migrations/7_deploy_v2_2.js +++ b/migrations/7_deploy_v2_2.js @@ -54,5 +54,5 @@ module.exports = async (deployer, network) => { ); await fiatTokenV2_2.initializeV2(""); await fiatTokenV2_2.initializeV2_1(THROWAWAY_ADDRESS); - await fiatTokenV2_2.initializeV2_2(); + await fiatTokenV2_2.initializeV2_2([]); }; diff --git a/migrations/8_deploy_v2_2_upgrader.js b/migrations/8_deploy_v2_2_upgrader.js index b6130d3af..b16161de4 100644 --- a/migrations/8_deploy_v2_2_upgrader.js +++ b/migrations/8_deploy_v2_2_upgrader.js @@ -1,6 +1,7 @@ const fs = require("fs"); const path = require("path"); const some = require("lodash/some"); +const { readBlacklistFile } = require("../utils"); const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); const FiatTokenProxy = artifacts.require("FiatTokenProxy"); @@ -18,7 +19,20 @@ if (fs.existsSync(path.join(__dirname, "..", "config.js"))) { } module.exports = async (deployer, network) => { - if (some(["development", "coverage"], (v) => network.includes(v))) { + const isTestEnvironment = some(["development", "coverage"], (v) => + network.includes(v) + ); + + // Proceed if and only if the blacklist file exists. + const accountsToBlacklist = readBlacklistFile( + path.join( + __dirname, + "..", + isTestEnvironment ? "blacklist.test.js" : "blacklist.remote.js" + ) + ); + + if (isTestEnvironment) { // DO NOT USE THESE ADDRESSES IN PRODUCTION proxyAdminAddress = "0x2F560290FEF1B3Ada194b6aA9c40aa71f8e95598"; proxyContractAddress = (await FiatTokenProxy.deployed()).address; @@ -42,7 +56,8 @@ module.exports = async (deployer, network) => { V2_2Upgrader, proxyContractAddress, fiatTokenV2_2.address, - proxyAdminAddress + proxyAdminAddress, + accountsToBlacklist ); console.log( diff --git a/package.json b/package.json index 1f12dc469..3081d88af 100644 --- a/package.json +++ b/package.json @@ -8,10 +8,11 @@ }, "scripts": { "compile": "truffle compile", - "contract-size": "truffle run contract-size", + "contract-size": "yarn contract-size:all --contracts $(find ./contracts -name \"*.sol\" -not -path \"./contracts/test/*\" | xargs -I {} /bin/bash -c 'basename {} .sol')", + "contract-size:all": "yarn compile && truffle run contract-size --checkMaxSize", "coverage": "truffle run coverage", "fmt": "prettier --write './**/*.sol' './**/*.js' './**/*.ts' './**/*.json' './**/*.md'", - "ganache": "ganache-cli --accounts=15 --deterministic --defaultBalanceEther=1000000 --quiet", + "ganache": "ganache-cli --accounts=15 --deterministic --defaultBalanceEther=1000000 --allowUnlimitedContractSize --gasLimit=0x1C9C380 --quiet", "lint": "eslint --ext '.js,.ts' './**/*.{j,t}s'", "migrate": "truffle migrate --interactive", "prepare": "husky install", diff --git a/scripts/validateAccountsToBlacklist.js b/scripts/validateAccountsToBlacklist.js new file mode 100644 index 000000000..4ef9125e8 --- /dev/null +++ b/scripts/validateAccountsToBlacklist.js @@ -0,0 +1,126 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +const path = require("path"); +const { readBlacklistFile } = require("../utils"); +const { toLower } = require("lodash"); + +const FiatTokenProxy = artifacts.require("FiatTokenProxy"); +const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); +const V2_2Upgrader = artifacts.require("V2_2Upgrader"); + +/** + * A utility script to validate that + * 1. V2_2Upgrader.accountsToBlacklist() values match with the list of addresses in blacklist.remote.js + * 2. The list of addresses in blacklist.remote.js are currently blacklisted. + * @param {string} proxyAddress the contract address of FiatTokenProxy + * @param {string} v2_2UpgraderAddress the contract address of V2_2Upgrader + */ +async function main(proxyAddress, v2_2UpgraderAddress) { + console.log("Comparing local state with deployed V2_2Upgrader..."); + const expectedAccountsToBlacklist = readBlacklistFile( + path.join(__dirname, "..", "blacklist.remote.js") + ).map(toLower); + console.log( + `>> Expecting ${expectedAccountsToBlacklist.length} accounts to blacklist` + ); + + const v2_2Upgrader = await V2_2Upgrader.at(v2_2UpgraderAddress); + const actualAccountsToBlacklist = ( + await v2_2Upgrader.accountsToBlacklist() + ).map(toLower); + console.log( + `>> Retrieved ${actualAccountsToBlacklist.length} accounts to blacklist from v2_2Upgrader` + ); + + console.log(">> Verifying accounts to blacklist..."); + for (const expectedAccount of expectedAccountsToBlacklist) { + if (!actualAccountsToBlacklist.includes(expectedAccount)) { + throw new Error( + `Expected account '${expectedAccount}' not found in v2_2Upgrader accountsToBlacklist` + ); + } + } + + for (const actualAccount of actualAccountsToBlacklist) { + if (!expectedAccountsToBlacklist.includes(actualAccount)) { + throw new Error( + `Actual account '${actualAccount}' not found in blacklist.remote.js` + ); + } + } + console.log(">> All accounts verified!"); + + console.log("Comparing local state with deployed FiatTokenProxy..."); + console.log(">> Validating that all accountsToBlacklist are blacklisted"); + const proxyAsV2_1 = await FiatTokenV2_1.at(proxyAddress); + + for (const account of expectedAccountsToBlacklist) { + if (!(await proxyAsV2_1.isBlacklisted(account))) { + throw new Error(`Account '${account}' is not currently blacklisted!`); + } + } + console.log(">> All accounts verified!"); +} + +module.exports = async (callback) => { + /* eslint-disable no-undef -- Config is a global variable in a truffle exec script https://github.com/trufflesuite/truffle/pull/3233 */ + const network = config.network; + const argv = config._; + /* eslint-enable no-undef */ + const usageError = new Error( + "Usage: yarn truffle exec scripts/validateAccountsToBlacklist.js [<0x-stripped Proxy address>] [<0x-stripped V2_2Upgrader address>] [--network=]" + ); + + // Truffle exec seems to auto parse a hex string passed in arguments into decimals. + // We need to strip the 0x in arguments to prevent this from happening. + const rawProxyAddress = argv[1]; + const rawV2_2UpgraderAddress = argv[2]; + const proxyAddress = + network === "development" && !rawProxyAddress + ? (await FiatTokenProxy.deployed()).address + : `0x${rawProxyAddress}`; + const v2_2UpgraderAddress = + network === "development" && !rawV2_2UpgraderAddress + ? (await V2_2Upgrader.deployed()).address + : `0x${rawV2_2UpgraderAddress}`; + + console.log(`network: ${network}`); + console.log(`proxyAddress: ${proxyAddress}`); + console.log(`v2_2UpgraderAddress: ${v2_2UpgraderAddress}`); + + if ( + !web3.utils.isAddress(proxyAddress) || + !web3.utils.isAddress(v2_2UpgraderAddress) + ) { + callback(usageError); + } else { + try { + await main(proxyAddress, v2_2UpgraderAddress); + callback(); + } catch (e) { + callback(e); + } + } +}; diff --git a/test/helpers/constants.ts b/test/helpers/constants.ts index bad7c971f..d97265ea5 100644 --- a/test/helpers/constants.ts +++ b/test/helpers/constants.ts @@ -1,9 +1,16 @@ +import BN from "bn.js"; + +export const BLOCK_GAS_LIMIT = 30e6; + export const ZERO_ADDRESS = "0x0000000000000000000000000000000000000000"; export const ZERO_BYTES32 = "0x0000000000000000000000000000000000000000000000000000000000000000"; export const MAX_UINT256 = "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; +export const POW_2_255_HEX = + "0x8000000000000000000000000000000000000000000000000000000000000000"; +export const POW_2_255_BN = new BN(POW_2_255_HEX.slice(2), 16); // derived from mnemonic: clarify final village pulse require old seek excite mushroom forest satoshi video export const ACCOUNTS_AND_KEYS: { address: string; key: string }[] = [ diff --git a/test/helpers/index.ts b/test/helpers/index.ts index f5c905332..d8a8e0594 100644 --- a/test/helpers/index.ts +++ b/test/helpers/index.ts @@ -9,6 +9,7 @@ import { FiatTokenV22Instance, FiatTokenV2Instance, } from "../../@types/generated"; +import _ from "lodash"; const FiatTokenV1 = artifacts.require("FiatTokenV1"); const FiatTokenV2 = artifacts.require("FiatTokenV2"); @@ -105,6 +106,15 @@ export function makeDomainSeparator( ); } +/** + * Helper function to generate a number of fake accounts. + * @param n the number of accounts to generate. + * @returns a list of accounts. + */ +export function generateAccounts(n: number): string[] { + return _.range(0, n).map(() => web3.eth.accounts.create().address); +} + export async function initializeToVersion( proxyOrImplementation: | FiatTokenProxyInstance @@ -115,7 +125,8 @@ export async function initializeToVersion( | FiatTokenV22Instance, version: "1" | "1.1" | "2" | "2.1" | "2.2", fiatTokenOwner: string, - lostAndFound: string + lostAndFound: string, + accountsToBlacklist: string[] = [] ): Promise { const proxyAsV1 = await FiatTokenV1.at(proxyOrImplementation.address); await proxyAsV1.initialize( @@ -143,6 +154,6 @@ export async function initializeToVersion( if (version >= "2.2") { const proxyAsV2_2 = await FiatTokenV2_2.at(proxyOrImplementation.address); - await proxyAsV2_2.initializeV2_2(); + await proxyAsV2_2.initializeV2_2(accountsToBlacklist); } } diff --git a/test/helpers/storageSlots.behavior.ts b/test/helpers/storageSlots.behavior.ts index db6542ec9..ca1c90e70 100644 --- a/test/helpers/storageSlots.behavior.ts +++ b/test/helpers/storageSlots.behavior.ts @@ -1,5 +1,6 @@ import BN from "bn.js"; import { FiatTokenProxyInstance } from "../../@types/generated"; +import { POW_2_255_BN } from "./constants"; const FiatTokenProxy = artifacts.require("FiatTokenProxy"); const FiatTokenV1 = artifacts.require("FiatTokenV1"); @@ -7,6 +8,11 @@ const FiatTokenV1_1 = artifacts.require("FiatTokenV1_1"); const FiatTokenV2 = artifacts.require("FiatTokenV2"); const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); +export const STORAGE_SLOT_NUMBERS = { + _deprecatedBlacklisted: 3, + balanceAndBlacklistStates: 9, +}; + export function usesOriginalStorageSlotPositions< T extends Truffle.ContractInstance >({ @@ -15,7 +21,7 @@ export function usesOriginalStorageSlotPositions< accounts, }: { Contract: Truffle.Contract; - version: 1 | 1.1 | 2 | 2.1; + version: 1 | 1.1 | 2 | 2.1 | 2.2; accounts: Truffle.Accounts; }): void { describe("uses original storage slot positions", () => { @@ -26,6 +32,10 @@ export function usesOriginalStorageSlotPositions< 30e6, 10e6, ]; + const [mintedBN, transferredBN] = [minted, transferred].map( + (v) => new BN(v, 10) + ); + const [ owner, proxyAdmin, @@ -67,6 +77,7 @@ export function usesOriginalStorageSlotPositions< await proxyAsFiatTokenV1.mint(alice, minted, { from: minter }); await proxyAsFiatTokenV1.transfer(bob, transferred, { from: alice }); await proxyAsFiatTokenV1.approve(charlie, allowance, { from: alice }); + await proxyAsFiatTokenV1.blacklist(bob, { from: blacklister }); await proxyAsFiatTokenV1.blacklist(charlie, { from: blacklister }); await proxyAsFiatTokenV1.pause({ from: pauser }); @@ -145,6 +156,7 @@ export function usesOriginalStorageSlotPositions< expect(parseAddress(slot)).to.equal(rescuer); }); } + if (version >= 2) { it("retains slot 15 for DOMAIN_SEPARATOR", async () => { const slot = await readSlot(proxy.address, 15); @@ -157,33 +169,88 @@ export function usesOriginalStorageSlotPositions< it("retains original storage slots for _deprecatedBlacklisted mapping", async () => { // _deprecatedBlacklisted[alice] let v = parseInt( - await readSlot(proxy.address, addressMappingSlot(alice, 3)), + await readSlot( + proxy.address, + addressMappingSlot(alice, STORAGE_SLOT_NUMBERS._deprecatedBlacklisted) + ), 16 ); expect(v).to.equal(0); - // _deprecatedBlacklisted[charlie] + // _deprecatedBlacklisted[bob] - this should be set to true in pre-v2.2 versions, + // and left untouched in v2.2+ versions. v = parseInt( - await readSlot(proxy.address, addressMappingSlot(charlie, 3)), + await readSlot( + proxy.address, + addressMappingSlot(bob, STORAGE_SLOT_NUMBERS._deprecatedBlacklisted) + ), 16 ); - expect(v).to.equal(1); - }); + if (version >= 2.2) { + expect(v).to.equal(0); + } else { + expect(v).to.equal(1); + } - it("retains original storage slots for balanceAndBlacklistStates mapping", async () => { - // balanceAndBlacklistStates[alice] - let v = parseInt( - await readSlot(proxy.address, addressMappingSlot(alice, 9)), + // _deprecatedBlacklisted[charlie] - this should be set to true in pre-v2.2 versions, + // and left untouched in v2.2+ versions. + v = parseInt( + await readSlot( + proxy.address, + addressMappingSlot( + charlie, + STORAGE_SLOT_NUMBERS._deprecatedBlacklisted + ) + ), 16 ); - expect(v).to.equal(minted - transferred); + if (version >= 2.2) { + expect(v).to.equal(0); + } else { + expect(v).to.equal(1); + } + }); - // balanceAndBlacklistStates[bob] - v = parseInt( - await readSlot(proxy.address, addressMappingSlot(bob, 9)), - 16 + it("retains original storage slots for balanceAndBlacklistStates mapping", async () => { + // balanceAndBlacklistStates[alice] - not blacklisted, has balance + let v = parseUint( + await readSlot( + proxy.address, + addressMappingSlot( + alice, + STORAGE_SLOT_NUMBERS.balanceAndBlacklistStates + ) + ) + ); + let expectedValue = mintedBN.sub(transferredBN); + expect(v.eq(expectedValue)).to.be.true; + + // balanceAndBlacklistStates[bob] - blacklisted, has balance + v = parseUint( + await readSlot( + proxy.address, + addressMappingSlot( + bob, + STORAGE_SLOT_NUMBERS.balanceAndBlacklistStates + ) + ) + ); + expectedValue = + version >= 2.2 ? POW_2_255_BN.add(transferredBN) : transferredBN; + expect(v.eq(expectedValue)).to.be.true; + + // balanceAndBlacklistStates[charlie] - blacklisted, no balance + v = parseUint( + await readSlot( + proxy.address, + addressMappingSlot( + charlie, + STORAGE_SLOT_NUMBERS.balanceAndBlacklistStates + ) + ) ); - expect(v).to.equal(transferred); + expectedValue = version >= 2.2 ? POW_2_255_BN : new BN(0); + expect(v.eq(expectedValue)).to.be.true; }); it("retains original storage slots for allowed mapping", async () => { @@ -235,7 +302,8 @@ export function usesOriginalStorageSlotPositions< }); } -async function readSlot( +// TODO: Organize these storage reading logic into a test helper. +export async function readSlot( address: string, slot: number | string ): Promise { @@ -255,7 +323,7 @@ function parseString(hex: string): string { return Buffer.from(hex.slice(0, len), "hex").toString("utf8"); } -function parseUint(hex: string): BN { +export function parseUint(hex: string): BN { return new BN(hex, 16); } @@ -267,7 +335,7 @@ function encodeAddress(addr: string): string { return addr.replace(/^0x/, "").toLowerCase().padStart(64, "0"); } -function addressMappingSlot(addr: string, pos: number): string { +export function addressMappingSlot(addr: string, pos: number): string { return web3.utils.keccak256("0x" + encodeAddress(addr) + encodeUint(pos)); } diff --git a/test/v1/events.test.js b/test/v1/events.test.js index 4926d3690..c83c05cb6 100644 --- a/test/v1/events.test.js +++ b/test/v1/events.test.js @@ -25,14 +25,14 @@ const { checkBlacklisterChangedEvent, checkUpgradeEvent, checkAdminChangedEvent, - UpgradedFiatTokenNewFields, + deployUpgradedFiatTokenNewFields, checkPauseEvent, checkTransferEvents, } = require("./helpers/tokenTest"); const amount = 100; -function runTests(_newToken, _accounts) { +function runTests(_newToken, _accounts, version) { let proxy, token; beforeEach(async () => { @@ -110,7 +110,7 @@ function runTests(_newToken, _accounts) { }); it("et008 should check Upgraded event", async () => { - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const initializeData = encodeCall( "initV2", ["bool", "address", "uint256"], diff --git a/test/v1/extendedPositive.test.js b/test/v1/extendedPositive.test.js index 8603290f0..e3fea983e 100644 --- a/test/v1/extendedPositive.test.js +++ b/test/v1/extendedPositive.test.js @@ -10,13 +10,13 @@ const { minterAccount, pauserAccount, initializeTokenWithProxy, - UpgradedFiatToken, + deployUpgradedFiatToken, upgradeTo, } = require("./helpers/tokenTest"); const amount = 100; -function runTests(newToken, _accounts) { +function runTests(newToken, _accounts, version) { let proxy, token; beforeEach(async () => { @@ -112,7 +112,7 @@ function runTests(newToken, _accounts) { }); it("ept008 should upgrade while paused", async () => { - const newRawToken = await UpgradedFiatToken.new(); + const newRawToken = await deployUpgradedFiatToken(version); await token.pause({ from: pauserAccount }); const tokenConfig = await upgradeTo(proxy, newRawToken); const newProxiedToken = tokenConfig.token; @@ -248,7 +248,7 @@ function runTests(newToken, _accounts) { it("ept022 should upgrade when msg.sender blacklisted", async () => { await token.blacklist(upgraderAccount, { from: blacklisterAccount }); - const newRawToken = await UpgradedFiatToken.new(); + const newRawToken = await deployUpgradedFiatToken(version); const tokenConfig = await upgradeTo(proxy, newRawToken); const newProxiedToken = tokenConfig.token; @@ -260,7 +260,7 @@ function runTests(newToken, _accounts) { }); it("ept023 should upgrade to blacklisted address", async () => { - const newRawToken = await UpgradedFiatToken.new(); + const newRawToken = await deployUpgradedFiatToken(version); await token.blacklist(newRawToken.address, { from: blacklisterAccount }); const tokenConfig = await upgradeTo(proxy, newRawToken); diff --git a/test/v1/helpers/tokenTest.js b/test/v1/helpers/tokenTest.js index d92ab618f..33dff3948 100644 --- a/test/v1/helpers/tokenTest.js +++ b/test/v1/helpers/tokenTest.js @@ -6,9 +6,14 @@ const Q = require("q"); const FiatTokenV1 = artifacts.require("FiatTokenV1"); const UpgradedFiatToken = artifacts.require("UpgradedFiatToken"); +const SignatureChecker = artifacts.require("SignatureChecker"); +const UpgradedFiatTokenV2_2 = artifacts.require("UpgradedFiatTokenV2_2"); const UpgradedFiatTokenNewFields = artifacts.require( "UpgradedFiatTokenNewFieldsTest" ); +const UpgradedFiatTokenV2_2NewFields = artifacts.require( + "UpgradedFiatTokenV2_2NewFieldsTest" +); const UpgradedFiatTokenNewFieldsNewLogic = artifacts.require( "UpgradedFiatTokenNewFieldsNewLogicTest" ); @@ -959,10 +964,31 @@ async function getInitializedV1(token) { return initialized; } +async function deployUpgradedFiatToken(version) { + if (version < 2.2) { + return UpgradedFiatToken.new(); + } else { + await SignatureChecker.new(); + UpgradedFiatTokenV2_2.link(SignatureChecker); + return UpgradedFiatTokenV2_2.new(); + } +} + +async function deployUpgradedFiatTokenNewFields(version) { + if (version < 2.2) { + return UpgradedFiatTokenNewFields.new(); + } else { + await SignatureChecker.new(); + UpgradedFiatTokenV2_2NewFields.link(SignatureChecker); + return UpgradedFiatTokenV2_2NewFields.new(); + } +} + module.exports = { FiatTokenV1, FiatTokenProxy, UpgradedFiatToken, + UpgradedFiatTokenV2_2, UpgradedFiatTokenNewFields, UpgradedFiatTokenNewFieldsNewLogic, name, @@ -993,6 +1019,8 @@ module.exports = { buildExpectedState, checkVariables, setMinter, + deployUpgradedFiatToken, + deployUpgradedFiatTokenNewFields, mint, burn, mintRaw, diff --git a/test/v1/helpers/wrapTests.js b/test/v1/helpers/wrapTests.js index 34f2bdc14..ed0a88826 100644 --- a/test/v1/helpers/wrapTests.js +++ b/test/v1/helpers/wrapTests.js @@ -1,34 +1,30 @@ const FiatTokenV1 = artifacts.require("FiatTokenV1"); const FiatTokenV1_1 = artifacts.require("FiatTokenV1_1"); const FiatTokenV2 = artifacts.require("FiatTokenV2"); - -// The following helpers make fresh original/upgraded tokens before each test. - -function newFiatTokenV1() { - return FiatTokenV1.new(); -} - -function newFiatTokenV1_1() { - return FiatTokenV1_1.new(); -} - -function newFiatTokenV2() { - return FiatTokenV2.new(); -} +const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); +const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); // Executes the run_tests_function using an original and // an upgraded token. The test_suite_name is printed standard output. function wrapTests(testSuiteName, runTestsFunction) { contract(`FiatTokenV1: ${testSuiteName}`, (accounts) => { - runTestsFunction(newFiatTokenV1, accounts); + runTestsFunction(FiatTokenV1.new, accounts, 1); }); contract(`FiatTokenV1_1: ${testSuiteName}`, (accounts) => { - runTestsFunction(newFiatTokenV1_1, accounts); + runTestsFunction(FiatTokenV1_1.new, accounts, 1.1); }); contract(`FiatTokenV2: ${testSuiteName}`, (accounts) => { - runTestsFunction(newFiatTokenV2, accounts); + runTestsFunction(FiatTokenV2.new, accounts, 2); + }); + + contract(`FiatTokenV2_1: ${testSuiteName}`, (accounts) => { + runTestsFunction(FiatTokenV2_1.new, accounts, 2.1); + }); + + contract(`FiatTokenV2_2: ${testSuiteName}`, (accounts) => { + runTestsFunction(FiatTokenV2_2.new, accounts, 2.2); }); } diff --git a/test/v1/misc.test.js b/test/v1/misc.test.js index 9108a9a2e..00894ec1b 100644 --- a/test/v1/misc.test.js +++ b/test/v1/misc.test.js @@ -1,4 +1,5 @@ const BN = require("bn.js"); +const { POW_2_255_HEX, POW_2_255_BN } = require("../helpers/constants"); const wrapTests = require("./helpers/wrapTests"); const { checkVariables, @@ -15,12 +16,18 @@ const { FiatTokenProxy, } = require("./helpers/tokenTest"); +// TODO: Change these to UPPERCASE const maxAmount = "0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; const maxAmountBN = new BN(maxAmount.slice(2), 16); + +const pow2_255Minus1Hex = + "0x7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"; +const pow2_255Minus1BN = new BN(pow2_255Minus1Hex.slice(2), 16); + const amount = 100; -function runTests(newToken, _accounts) { +function runTests(newToken, _accounts, version) { let proxy, token; beforeEach(async () => { @@ -736,20 +743,27 @@ function runTests(newToken, _accounts) { await checkVariables([token], [customVars]); }); - it("ms048 mint works on amount=2^256-1", async () => { - await token.configureMinter(minterAccount, maxAmount, { + it(`ms048 mint works on amount=${ + version < 2.2 ? "2^256-1" : "2^255-1" + }`, async () => { + const [amount, amountBN] = + version < 2.2 + ? [maxAmount, maxAmountBN] + : [pow2_255Minus1Hex, pow2_255Minus1BN]; + + await token.configureMinter(minterAccount, amount, { from: masterMinterAccount, }); let customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "minterAllowance.minterAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, ]; await checkVariables([token], [customVars]); - await token.mint(arbitraryAccount, maxAmount, { from: minterAccount }); + await token.mint(arbitraryAccount, amount, { from: minterAccount }); customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { @@ -758,29 +772,69 @@ function runTests(newToken, _accounts) { }, { variable: "balanceAndBlacklistStates.arbitraryAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, ]; await checkVariables([token], [customVars]); }); - it("ms049 burn on works on amount=2^256-1", async () => { - await token.configureMinter(minterAccount, maxAmount, { + if (version >= 2.2) { + it("ms048(b) mint does not work on amount=2^255", async () => { + await token.configureMinter(minterAccount, POW_2_255_HEX, { + from: masterMinterAccount, + }); + let customVars = [ + { variable: "isAccountMinter.minterAccount", expectedValue: true }, + { + variable: "minterAllowance.minterAccount", + expectedValue: POW_2_255_BN, + }, + ]; + await checkVariables([token], [customVars]); + + await expectRevert( + token.mint(arbitraryAccount, POW_2_255_HEX, { from: minterAccount }) + ); + customVars = [ + { variable: "isAccountMinter.minterAccount", expectedValue: true }, + { + variable: "minterAllowance.minterAccount", + expectedValue: POW_2_255_BN, + }, + { + variable: "balanceAndBlacklistStates.arbitraryAccount", + expectedValue: new BN(0), + }, + { variable: "totalSupply", expectedValue: new BN(0) }, + ]; + await checkVariables([token], [customVars]); + }); + } + + it(`ms049 burn on works on amount=${ + version < 2.2 ? "2^256-1" : "2^255-1" + }`, async () => { + const [amount, amountBN] = + version < 2.2 + ? [maxAmount, maxAmountBN] + : [pow2_255Minus1Hex, pow2_255Minus1BN]; + + await token.configureMinter(minterAccount, amount, { from: masterMinterAccount, }); - await token.mint(minterAccount, maxAmount, { from: minterAccount }); + await token.mint(minterAccount, amount, { from: minterAccount }); let customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.minterAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, ]; await checkVariables([token], [customVars]); - await token.burn(maxAmount, { from: minterAccount }); + await token.burn(amount, { from: minterAccount }); customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, ]; @@ -788,17 +842,29 @@ function runTests(newToken, _accounts) { }); it("ms050 approve works on amount=2^256-1", async () => { - await token.configureMinter(minterAccount, maxAmount, { + const [mintAmount, mintAmountBN] = + version < 2.2 + ? [maxAmount, maxAmountBN] + : [pow2_255Minus1Hex, pow2_255Minus1BN]; + + await token.configureMinter(minterAccount, mintAmount, { from: masterMinterAccount, }); - await token.mint(arbitraryAccount, maxAmount, { from: minterAccount }); + await token.mint(arbitraryAccount, mintAmount, { + from: minterAccount, + }); let customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.arbitraryAccount", - expectedValue: maxAmountBN, + expectedValue: mintAmountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { + variable: "balanceAndBlacklistStates.pauserAccount", + // TODO: Abstract all new BN(0) static var. + expectedValue: new BN(0), + }, + { variable: "totalSupply", expectedValue: mintAmountBN }, ]; await checkVariables([token], [customVars]); @@ -807,9 +873,13 @@ function runTests(newToken, _accounts) { { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.arbitraryAccount", - expectedValue: maxAmountBN, + expectedValue: mintAmountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { + variable: "balanceAndBlacklistStates.pauserAccount", + expectedValue: new BN(0), + }, + { variable: "totalSupply", expectedValue: mintAmountBN }, { variable: "allowance.arbitraryAccount.pauserAccount", expectedValue: maxAmountBN, @@ -818,63 +888,83 @@ function runTests(newToken, _accounts) { await checkVariables([token], [customVars]); }); - it("ms051 transfer works on amount=2^256-1", async () => { - await token.configureMinter(minterAccount, maxAmount, { + it(`ms051 transfer works on amount=${ + version < 2.2 ? "2^256-1" : "2^255-1" + }`, async () => { + const [amount, amountBN] = + version < 2.2 + ? [maxAmount, maxAmountBN] + : [pow2_255Minus1Hex, pow2_255Minus1BN]; + + await token.configureMinter(minterAccount, amount, { from: masterMinterAccount, }); - await token.mint(arbitraryAccount, maxAmount, { from: minterAccount }); + await token.mint(arbitraryAccount, amount, { from: minterAccount }); let customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.arbitraryAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, ]; await checkVariables([token], [customVars]); - await token.transfer(pauserAccount, maxAmount, { from: arbitraryAccount }); + await token.transfer(pauserAccount, amount, { + from: arbitraryAccount, + }); customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.pauserAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, ]; await checkVariables([token], [customVars]); }); - it("ms052 transferFrom works on amount=2^256-1", async () => { - await token.configureMinter(minterAccount, maxAmount, { + it(`ms052 transferFrom works on amount=${ + version < 2.2 ? "2^256-1" : "2^255-1" + }`, async () => { + const [amount, amountBN] = + version < 2.2 + ? [maxAmount, maxAmountBN] + : [pow2_255Minus1Hex, pow2_255Minus1BN]; + + await token.configureMinter(minterAccount, amount, { from: masterMinterAccount, }); - await token.mint(arbitraryAccount, maxAmount, { from: minterAccount }); - await token.approve(pauserAccount, maxAmount, { from: arbitraryAccount }); + await token.mint(arbitraryAccount, amount, { + from: minterAccount, + }); + await token.approve(pauserAccount, amount, { + from: arbitraryAccount, + }); let customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.arbitraryAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, { variable: "allowance.arbitraryAccount.pauserAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, ]; await checkVariables([token], [customVars]); - await token.transferFrom(arbitraryAccount, pauserAccount, maxAmount, { + await token.transferFrom(arbitraryAccount, pauserAccount, amount, { from: pauserAccount, }); customVars = [ { variable: "isAccountMinter.minterAccount", expectedValue: true }, { variable: "balanceAndBlacklistStates.pauserAccount", - expectedValue: maxAmountBN, + expectedValue: amountBN, }, - { variable: "totalSupply", expectedValue: maxAmountBN }, + { variable: "totalSupply", expectedValue: amountBN }, ]; await checkVariables([token], [customVars]); }); diff --git a/test/v1/proxyNegative.test.js b/test/v1/proxyNegative.test.js index 8cd5f15ef..72869f543 100644 --- a/test/v1/proxyNegative.test.js +++ b/test/v1/proxyNegative.test.js @@ -20,12 +20,12 @@ const { encodeCall, FiatTokenV1, UpgradedFiatToken, - UpgradedFiatTokenNewFields, + deployUpgradedFiatTokenNewFields, } = require("./helpers/tokenTest"); const amount = 100; -function runTests(newToken, _accounts) { +function runTests(newToken, _accounts, version) { let rawToken, proxy, token; beforeEach(async () => { @@ -134,7 +134,7 @@ function runTests(newToken, _accounts) { }); it("nut010 should fail to call updateToAndCall with non-adminAccount", async () => { - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const initializeData = encodeCall( "initialize", ["bool", "address", "uint256"], @@ -164,7 +164,7 @@ function runTests(newToken, _accounts) { await token.mint(arbitraryAccount, mintAmount, { from: minterAccount }); await token.transfer(pauserAccount, mintAmount, { from: arbitraryAccount }); - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const data = encodeCall( "initialize", [ diff --git a/test/v1/proxyPositive.test.js b/test/v1/proxyPositive.test.js index dc8acfc8a..d952dedd6 100644 --- a/test/v1/proxyPositive.test.js +++ b/test/v1/proxyPositive.test.js @@ -25,12 +25,13 @@ const { UpgradedFiatTokenNewFields, UpgradedFiatTokenNewFieldsNewLogic, getAdmin, + deployUpgradedFiatTokenNewFields, } = require("./helpers/tokenTest"); const { makeRawTransaction, sendRawTransaction } = require("./helpers/abi"); const amount = 100; -function runTests(newToken, _accounts) { +function runTests(newToken, _accounts, version) { let rawToken, proxy, token; beforeEach(async () => { @@ -86,7 +87,7 @@ function runTests(newToken, _accounts) { await token.mint(arbitraryAccount, mintAmount, { from: minterAccount }); await token.transfer(pauserAccount, mintAmount, { from: arbitraryAccount }); - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const initializeData = encodeCall( "initV2", ["bool", "address", "uint256"], @@ -175,7 +176,7 @@ function runTests(newToken, _accounts) { }); it("upt008 should deploy upgraded version of contract with new data fields and without previous deployment and ensure new fields correct", async () => { - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const newProxy = await FiatTokenProxy.new(upgradedToken.address, { from: proxyOwnerAccount, }); @@ -403,7 +404,7 @@ function runTests(newToken, _accounts) { it("upt011 should upgradeToAndCall while paused and upgraded contract should be paused as a result", async () => { await token.pause({ from: pauserAccount }); - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const initializeData = encodeCall( "initV2", ["bool", "address", "uint256"], @@ -426,7 +427,7 @@ function runTests(newToken, _accounts) { it("upt012 should upgradeToAndCall while upgrader is blacklisted", async () => { await token.blacklist(proxyOwnerAccount, { from: blacklisterAccount }); - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); const initializeData = encodeCall( "initV2", ["bool", "address", "uint256"], @@ -445,7 +446,7 @@ function runTests(newToken, _accounts) { }); it("upt013 should upgradeToAndCall while new logic is blacklisted", async () => { - const upgradedToken = await UpgradedFiatTokenNewFields.new(); + const upgradedToken = await deployUpgradedFiatTokenNewFields(version); await token.blacklist(upgradedToken.address, { from: blacklisterAccount }); const initializeData = encodeCall( diff --git a/test/v2/FiatTokenV2.test.ts b/test/v2/FiatTokenV2.test.ts index 092851cc6..9fc272560 100644 --- a/test/v2/FiatTokenV2.test.ts +++ b/test/v2/FiatTokenV2.test.ts @@ -40,6 +40,11 @@ contract("FiatTokenV2", (accounts) => { }); behavesLikeFiatTokenV2(accounts, () => fiatToken, fiatTokenOwner); + usesOriginalStorageSlotPositions({ + Contract: FiatTokenV2, + version: 2, + accounts, + }); }); export function behavesLikeFiatTokenV2( @@ -60,12 +65,6 @@ export function behavesLikeFiatTokenV2( behavesLikeRescuable(getFiatToken as () => RescuableInstance, accounts); - usesOriginalStorageSlotPositions({ - Contract: FiatTokenV2, - version: 2, - accounts, - }); - it("has the expected domain separator", async () => { expect(await getFiatToken().DOMAIN_SEPARATOR()).to.equal(domainSeparator); }); diff --git a/test/v2/FiatTokenV2_1.test.ts b/test/v2/FiatTokenV2_1.test.ts index 44f49c4ed..d4ec52077 100644 --- a/test/v2/FiatTokenV2_1.test.ts +++ b/test/v2/FiatTokenV2_1.test.ts @@ -1,5 +1,6 @@ import { FiatTokenV21Instance } from "../../@types/generated"; import { expectRevert } from "../helpers"; +import { usesOriginalStorageSlotPositions } from "../helpers/storageSlots.behavior"; import { behavesLikeFiatTokenV2 } from "./FiatTokenV2.test"; const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); @@ -24,6 +25,11 @@ contract("FiatTokenV2_1", (accounts) => { }); behavesLikeFiatTokenV2(accounts, () => fiatToken, fiatTokenOwner); + usesOriginalStorageSlotPositions({ + Contract: FiatTokenV2_1, + version: 2.1, + accounts, + }); describe("initializeV2_1", () => { const [, user, lostAndFound] = accounts; diff --git a/test/v2/FiatTokenV2_2.test.ts b/test/v2/FiatTokenV2_2.test.ts index 428aaf805..8a408323e 100644 --- a/test/v2/FiatTokenV2_2.test.ts +++ b/test/v2/FiatTokenV2_2.test.ts @@ -1,8 +1,28 @@ +import BN from "bn.js"; +import crypto from "crypto"; import { AnyFiatTokenV2Instance, FiatTokenV22InstanceExtended, } from "../../@types/AnyFiatTokenV2Instance"; -import { expectRevert, initializeToVersion } from "../helpers"; +import { + expectRevert, + generateAccounts, + hexStringFromBuffer, + initializeToVersion, + strip0x, +} from "../helpers"; +import { + ACCOUNTS_AND_KEYS, + MAX_UINT256, + POW_2_255_BN, +} from "../helpers/constants"; +import { + STORAGE_SLOT_NUMBERS, + addressMappingSlot, + parseUint, + readSlot, + usesOriginalStorageSlotPositions, +} from "../helpers/storageSlots.behavior"; import { behavesLikeFiatTokenV2, getERC1271Wallet } from "./FiatTokenV2.test"; import { hasGasAbstraction } from "./GasAbstraction/GasAbstraction.behavior"; import { @@ -11,18 +31,26 @@ import { makeDomainSeparator, permitSignature, permitSignatureV22, + prepareSignature, transferWithAuthorizationSignature, transferWithAuthorizationSignatureV22, cancelAuthorizationSignature, cancelAuthorizationSignatureV22, receiveWithAuthorizationSignature, receiveWithAuthorizationSignatureV22, + signTransferAuthorization, + signReceiveAuthorization, } from "./GasAbstraction/helpers"; +const FiatTokenProxy = artifacts.require("FiatTokenProxy"); +const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); contract("FiatTokenV2_2", (accounts) => { const fiatTokenOwner = accounts[9]; + const lostAndFound = accounts[2]; + const proxyOwnerAccount = accounts[14]; + let fiatToken: FiatTokenV22InstanceExtended; const getFiatToken = ( @@ -35,23 +63,152 @@ contract("FiatTokenV2_2", (accounts) => { }; beforeEach(async () => { - const [, , lostAndFound] = accounts; - fiatToken = await FiatTokenV2_2.new(); - await initializeToVersion(fiatToken, "2.2", fiatTokenOwner, lostAndFound); + await initializeToVersion(fiatToken, "2.1", fiatTokenOwner, lostAndFound); }); - behavesLikeFiatTokenV2( - accounts, - getFiatToken(SignatureBytesType.Unpacked), - fiatTokenOwner - ); + describe("initializeV2_2", () => { + it("disallows calling initializeV2_2 twice", async () => { + await fiatToken.initializeV2_2([]); + await expectRevert(fiatToken.initializeV2_2([])); + }); - behavesLikeFiatTokenV22( - accounts, - getFiatToken(SignatureBytesType.Packed), - fiatTokenOwner - ); + it("should blacklist all accountsToBlacklist", async () => { + const [unblacklistedAccount, ...accountsToBlacklist] = generateAccounts( + 10 + ); + + // Prepare a proxy that's tied to a V2_1 implementation so that we can blacklist + // the account in _deprecatedBlacklisted first. + const _fiatTokenV2_1 = await FiatTokenV2_1.new(); + const _proxy = await FiatTokenProxy.new(_fiatTokenV2_1.address, { + from: proxyOwnerAccount, + }); + const _proxyAsV2_1 = await FiatTokenV2_1.at(_proxy.address); + await initializeToVersion( + _proxyAsV2_1, + "2.1", + fiatTokenOwner, + lostAndFound + ); + await Promise.all( + accountsToBlacklist.map((a) => + _proxyAsV2_1.blacklist(a, { from: fiatTokenOwner }) + ) + ); + + // Sanity check that _deprecatedBlacklisted is set, and balanceAndBlacklistStates is not set for + // every accountsToBlacklist. + expect( + ( + await readDeprecatedBlacklisted(_proxy.address, accountsToBlacklist) + ).every((result) => result === 1) + ).to.be.true; + expect( + ( + await readBalanceAndBlacklistStates( + _proxy.address, + accountsToBlacklist + ) + ).every((result) => result.eq(new BN(0))) + ).to.be.true; + + // Sanity check that _deprecatedBlacklisted is set, and balanceAndBlacklistStates is not set + // for `address(this)` + expect( + (await readDeprecatedBlacklisted(_proxy.address, [_proxy.address]))[0] + ).to.equal(1); + expect( + ( + await readBalanceAndBlacklistStates(_proxy.address, [_proxy.address]) + )[0].eq(new BN(0)) + ).to.be.true; + + // Call the initializeV2_2 function through an upgrade call. + await _proxy.upgradeToAndCall( + fiatToken.address, + web3.eth.abi.encodeFunctionSignature("initializeV2_2(address[])") + + strip0x( + web3.eth.abi.encodeParameter("address[]", accountsToBlacklist) + ), + { from: proxyOwnerAccount } + ); + + // Validate that isBlacklisted returns true for every accountsToBlacklist. + const _proxyAsV2_2 = await FiatTokenV2_2.at(_proxy.address); + const areAccountsBlacklisted = await Promise.all( + accountsToBlacklist.map((account) => + _proxyAsV2_2.isBlacklisted(account) + ) + ); + expect(areAccountsBlacklisted.every((b) => b)).to.be.true; + + // Validate that _deprecatedBlacklisted is unset, and balanceAndBlacklistStates is set for every + // accountsToBlacklist. + expect( + ( + await readDeprecatedBlacklisted(_proxy.address, accountsToBlacklist) + ).every((result) => result === 0) + ).to.be.true; + expect( + ( + await readBalanceAndBlacklistStates( + _proxy.address, + accountsToBlacklist + ) + ).every((result) => result.eq(POW_2_255_BN)) + ).to.be.true; + + // Validate that _deprecatedBlacklisted is unset, and balanceAndBlacklistStates is set for + // `address(this)` + expect( + (await readDeprecatedBlacklisted(_proxy.address, [_proxy.address]))[0] + ).to.equal(0); + expect( + ( + await readBalanceAndBlacklistStates(_proxy.address, [_proxy.address]) + )[0].eq(POW_2_255_BN) + ).to.be.true; + + // Sanity check that an unblacklisted account does not get blacklisted. + expect(await _proxyAsV2_2.isBlacklisted(unblacklistedAccount)).to.be + .false; + }); + + it("should revert if an accountToBlacklist was not blacklisted", async () => { + const accountsToBlacklist = generateAccounts(1); + await expectRevert( + fiatToken.initializeV2_2(accountsToBlacklist), + "FiatTokenV2_2: Blacklisting previously unblacklisted account!" + ); + + // Sanity check that the account is not blacklisted after revert. + expect(await fiatToken.isBlacklisted(accountsToBlacklist[0])).to.be.false; + }); + }); + + describe("initialized contract", () => { + beforeEach(async () => { + await fiatToken.initializeV2_2([]); + }); + + behavesLikeFiatTokenV2( + accounts, + getFiatToken(SignatureBytesType.Unpacked), + fiatTokenOwner + ); + + behavesLikeFiatTokenV22( + accounts, + getFiatToken(SignatureBytesType.Packed), + fiatTokenOwner + ); + usesOriginalStorageSlotPositions({ + Contract: FiatTokenV2_2, + version: 2.2, + accounts, + }); + }); }); export function behavesLikeFiatTokenV22( @@ -59,14 +216,17 @@ export function behavesLikeFiatTokenV22( getFiatToken: () => AnyFiatTokenV2Instance, fiatTokenOwner: string ): void { + const [minter, arbitraryAccount] = accounts.slice(3); let domainSeparator: string; + let fiatToken: FiatTokenV22InstanceExtended; beforeEach(async () => { + fiatToken = getFiatToken() as FiatTokenV22InstanceExtended; domainSeparator = makeDomainSeparator( "USD Coin", "2", 1, // hardcoded to 1 because of ganache bug: https://github.com/trufflesuite/ganache/issues/1643 - getFiatToken().address + fiatToken.address ); }); @@ -78,7 +238,7 @@ export function behavesLikeFiatTokenV22( accounts, }; - // Test gas abstraction funtionalities with both EOA and AA wallets + // Test gas abstraction functionalities with both EOA and AA wallets hasGasAbstraction({ ...v22TestParams, signerWalletType: WalletType.EOA, @@ -90,12 +250,122 @@ export function behavesLikeFiatTokenV22( signatureBytesType: SignatureBytesType.Packed, }); - describe("initializeV2_2", () => { - it("disallows calling initializeV2_2 twice", async () => { + // Additional negative test cases. + describe("will trigger exceeded 2^255 balance error", () => { + const incrementAmount = 1000; + const recipient = arbitraryAccount; + const errorMessage = "FiatTokenV2_2: Balance exceeds (2^255 - 1)"; + + beforeEach(async () => { + const recipientInitialBalance = POW_2_255_BN.sub(new BN(incrementAmount)); + await fiatToken.configureMinter(minter, POW_2_255_BN, { + from: fiatTokenOwner, + }); + await fiatToken.mint(recipient, recipientInitialBalance, { + from: minter, + }); + expect((await fiatToken.balanceOf(recipient)).eq(recipientInitialBalance)) + .to.be.true; + }); + + it("should fail to mint to recipient if balance will exceed 2^255", async () => { + await expectRevert( + fiatToken.mint(recipient, incrementAmount, { from: minter }), + errorMessage + ); + }); + + it("should fail to transfer to recipient if balance will exceed 2^255", async () => { + await fiatToken.mint(minter, incrementAmount, { from: minter }); + await expectRevert( + fiatToken.transfer(recipient, incrementAmount, { from: minter }), + errorMessage + ); + }); + + it("should fail call transferFrom to recipient if balance will exceed 2^255", async () => { + await fiatToken.mint(minter, incrementAmount, { from: minter }); + await fiatToken.approve(arbitraryAccount, incrementAmount, { + from: minter, + }); + await expectRevert( - (getFiatToken() as FiatTokenV22InstanceExtended).initializeV2_2() + fiatToken.transferFrom(minter, recipient, incrementAmount, { + from: arbitraryAccount, + }), + errorMessage ); }); + + context("EIP3009", () => { + const signer = ACCOUNTS_AND_KEYS[0]; + const from = signer.address; + const to = recipient; + const value = incrementAmount; + const validAfter = 0; + const validBefore = MAX_UINT256; + const nonce = hexStringFromBuffer(crypto.randomBytes(32)); + + beforeEach(async () => { + await fiatToken.mint(signer.address, incrementAmount, { + from: minter, + }); + }); + + it("should fail to call transferWithAuthorization to recipient if balance will exceed 2^255", async () => { + const signature = signTransferAuthorization( + from, + to, + value, + validAfter, + validBefore, + nonce, + domainSeparator, + signer.key + ); + + await expectRevert( + fiatToken.transferWithAuthorization( + from, + to, + value, + validAfter, + validBefore, + nonce, + ...prepareSignature(signature, SignatureBytesType.Packed), + { from: to } + ), + errorMessage + ); + }); + + it("should fail to call receiveWithAuthorization to recipient if balance will exceed 2^255", async () => { + const signature = signReceiveAuthorization( + from, + to, + value, + validAfter, + validBefore, + nonce, + domainSeparator, + signer.key + ); + + await expectRevert( + fiatToken.receiveWithAuthorization( + from, + to, + value, + validAfter, + validBefore, + nonce, + ...prepareSignature(signature, SignatureBytesType.Packed), + { from: to } + ), + errorMessage + ); + }); + }); }); } @@ -136,3 +406,47 @@ export function initializeOverloadedMethods( fiatToken.methods[cancelAuthorizationSignatureV22]; } } + +/** + * Helper method to read the _deprecatedBlacklisted map. + * @param proxyOrImplementation the address of the proxy or implementation contract. + * @param accounts the accounts to read states for. + * @returns the results (in order) from reading the map. + */ +async function readDeprecatedBlacklisted( + proxyOrImplementation: string, + accounts: string[] +): Promise { + return ( + await Promise.all( + accounts.map((a) => + readSlot( + proxyOrImplementation, + addressMappingSlot(a, STORAGE_SLOT_NUMBERS._deprecatedBlacklisted) + ) + ) + ) + ).map((result) => parseInt(result, 16)); +} + +/** + * Helper method to read the balanceAndBlacklistStates map. + * @param proxyOrImplementation the address of the proxy or implementation contract. + * @param accounts the accounts to read states for. + * @returns the results (in order) from reading the map. + */ +async function readBalanceAndBlacklistStates( + proxyOrImplementation: string, + accounts: string[] +): Promise { + return ( + await Promise.all( + accounts.map((a) => + readSlot( + proxyOrImplementation, + addressMappingSlot(a, STORAGE_SLOT_NUMBERS.balanceAndBlacklistStates) + ) + ) + ) + ).map((result) => parseUint(result)); +} diff --git a/test/v2/MockFiatTokenWithEditableBalanceAndBlacklistStates.test.ts b/test/v2/MockFiatTokenWithEditableBalanceAndBlacklistStates.test.ts new file mode 100644 index 000000000..320028b3a --- /dev/null +++ b/test/v2/MockFiatTokenWithEditableBalanceAndBlacklistStates.test.ts @@ -0,0 +1,226 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +import BN from "bn.js"; +import { MockFiatTokenWithEditableBalanceAndBlacklistStatesInstance } from "../../@types/generated"; +import { expectRevert, initializeToVersion } from "../helpers"; +import { ACCOUNTS_AND_KEYS, POW_2_255_BN } from "../helpers/constants"; + +const SignatureChecker = artifacts.require("SignatureChecker"); +const MockFiatTokenWithEditableBalanceAndBlacklistStates = artifacts.require( + "MockFiatTokenWithEditableBalanceAndBlacklistStates" +); + +contract("MockFiatTokenWithEditableBalanceAndBlacklistStates", (accounts) => { + const fiatTokenOwner = accounts[9]; + const userOne = ACCOUNTS_AND_KEYS[0].address; + + let fiatToken: MockFiatTokenWithEditableBalanceAndBlacklistStatesInstance; + + const ZERO = new BN(0); + const SEVEN = new BN(7, 10); + + beforeEach(async () => { + await SignatureChecker.new(); + MockFiatTokenWithEditableBalanceAndBlacklistStates.link(SignatureChecker); + fiatToken = await MockFiatTokenWithEditableBalanceAndBlacklistStates.new(); + const [, , lostAndFound] = accounts; + await initializeToVersion(fiatToken, "2.2", fiatTokenOwner, lostAndFound); + }); + + async function expectBalanceAndBlacklistStatesToBe( + account: string, + expectedState: BN + ) { + const currentState = await fiatToken.getBalanceAndBlacklistStates(account); + expect(currentState.eq(expectedState)).to.be.true; + } + + describe("internal_setBlacklistState", () => { + context("when _shouldBlacklist is true", () => { + const _shouldBlacklist = true; + + it("should store 2^255 if the account was not blacklisted", async () => { + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + }); + + it("should retain 2^255 if the account was blacklisted", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, POW_2_255_BN); + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + }); + + it("should store (2^255 + previous balance) if the account has a balance", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, SEVEN); + await expectBalanceAndBlacklistStatesToBe(userOne, SEVEN); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe( + userOne, + POW_2_255_BN.add(SEVEN) + ); + }); + }); + + context("when _shouldBlacklist is false", () => { + const _shouldBlacklist = false; + + it("should store 0 if the account was blacklisted", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, POW_2_255_BN); + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + }); + + it("should retain 0 if the account was not blacklisted", async () => { + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + }); + + it("should store previous balance if the account has a balance", async () => { + await fiatToken.setBalanceAndBlacklistStates( + userOne, + POW_2_255_BN.add(SEVEN) + ); + await expectBalanceAndBlacklistStatesToBe( + userOne, + POW_2_255_BN.add(SEVEN) + ); + + await fiatToken.internal_setBlacklistState(userOne, _shouldBlacklist); + + await expectBalanceAndBlacklistStatesToBe(userOne, SEVEN); + }); + }); + }); + + describe("internal_setBalance", () => { + it("should revert if new balance is greater than or equal to 2^255", async () => { + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + + const newBalance = POW_2_255_BN; // 2^255 + await expectRevert( + fiatToken.internal_setBalance(userOne, newBalance), + "FiatTokenV2_2: Balance exceeds (2^255 - 1)" + ); + + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + }); + + it("should revert if the account is blacklisted", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, POW_2_255_BN); + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + + const newBalance = SEVEN; + await expectRevert( + fiatToken.internal_setBalance(userOne, newBalance), + "FiatTokenV2_2: Account is blacklisted" + ); + + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + }); + + it("should always reset balances to the _balance parameter if new balance is less than 2^255 and account is not blacklisted", async () => { + const newBalance = POW_2_255_BN.sub(new BN(1)); // 2^255 - 1 + await expectBalanceAndBlacklistStatesToBe(userOne, ZERO); + + // Set to some high value. + await fiatToken.internal_setBalance(userOne, newBalance); + await expectBalanceAndBlacklistStatesToBe(userOne, newBalance); + + // Then, choose a lower value, ensuring that it sets to the value. + await fiatToken.internal_setBalance(userOne, SEVEN); + await expectBalanceAndBlacklistStatesToBe(userOne, SEVEN); + + // Then, choose a higher value, ensuring that it sets to the value. + await fiatToken.internal_setBalance(userOne, newBalance); + await expectBalanceAndBlacklistStatesToBe(userOne, newBalance); + }); + }); + + describe("internal_isBlacklisted", () => { + it("should return false if the high bit is 0", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, SEVEN); + expect(await fiatToken.internal_isBlacklisted(userOne)).to.be.false; + }); + + it("should return true if the high bit is 1", async () => { + await fiatToken.setBalanceAndBlacklistStates( + userOne, + POW_2_255_BN.add(SEVEN) + ); + expect(await fiatToken.internal_isBlacklisted(userOne)).to.be.true; + }); + + it("should not change balanceAndBlacklistState", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, SEVEN); + await fiatToken.internal_isBlacklisted(userOne); + await expectBalanceAndBlacklistStatesToBe(userOne, SEVEN); + + await fiatToken.setBalanceAndBlacklistStates(userOne, POW_2_255_BN); + await fiatToken.internal_isBlacklisted(userOne); + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + }); + }); + + describe("internal_balanceOf", () => { + it("should return the correct balance when the high bit is 0", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, SEVEN); + expect((await fiatToken.internal_balanceOf(userOne)).eq(SEVEN)).to.be + .true; + }); + + it("should return the correct balance when the high bit is 1", async () => { + await fiatToken.setBalanceAndBlacklistStates( + userOne, + POW_2_255_BN.add(SEVEN) + ); + expect((await fiatToken.internal_balanceOf(userOne)).eq(SEVEN)).to.be + .true; + }); + + it("should not change balanceAndBlacklistState", async () => { + await fiatToken.setBalanceAndBlacklistStates(userOne, SEVEN); + await fiatToken.internal_balanceOf(userOne); + await expectBalanceAndBlacklistStatesToBe(userOne, SEVEN); + + await fiatToken.setBalanceAndBlacklistStates(userOne, POW_2_255_BN); + await fiatToken.internal_balanceOf(userOne); + await expectBalanceAndBlacklistStatesToBe(userOne, POW_2_255_BN); + }); + }); +}); diff --git a/test/v2/MockFiatTokenWithEditableChainId.test.ts b/test/v2/MockFiatTokenWithEditableChainId.test.ts index 31b58f4f0..86180790c 100644 --- a/test/v2/MockFiatTokenWithEditableChainId.test.ts +++ b/test/v2/MockFiatTokenWithEditableChainId.test.ts @@ -31,7 +31,7 @@ contract("MockFiatTokenWithEditableChainId", (accounts) => { await fiatToken.initializeV2("USD Coin", { from: fiatTokenOwner }); const [, , lostAndFound] = accounts; await fiatToken.initializeV2_1(lostAndFound); - await fiatToken.initializeV2_2(); + await fiatToken.initializeV2_2([]); }); describe("DOMAIN_SEPARATOR", () => { diff --git a/test/v2/V2_2Upgrader.test.ts b/test/v2/V2_2Upgrader.test.ts index 960e6483e..d97742b09 100644 --- a/test/v2/V2_2Upgrader.test.ts +++ b/test/v2/V2_2Upgrader.test.ts @@ -4,13 +4,24 @@ import { FiatTokenV22Instance, V22UpgraderInstance, } from "../../@types/generated"; -import { expectRevert, initializeToVersion } from "../helpers"; +import { + expectRevert, + generateAccounts, + initializeToVersion, +} from "../helpers"; +import { readBlacklistFile } from "../../utils"; +import path from "path"; +import { toLower } from "lodash"; +import { BLOCK_GAS_LIMIT } from "../helpers/constants"; const FiatTokenProxy = artifacts.require("FiatTokenProxy"); const FiatTokenV1_1 = artifacts.require("FiatTokenV1_1"); const FiatTokenV2_1 = artifacts.require("FiatTokenV2_1"); const FiatTokenV2_2 = artifacts.require("FiatTokenV2_2"); const V2_2Upgrader = artifacts.require("V2_2Upgrader"); +const accountsToBlacklist = readBlacklistFile( + path.join(__dirname, "..", "..", "blacklist.test.js") +); contract("V2_2Upgrader", (accounts) => { let fiatTokenProxy: FiatTokenProxyInstance; @@ -23,6 +34,7 @@ contract("V2_2Upgrader", (accounts) => { let v2_1MasterMinter: string; let originalProxyAdmin: string; + const blacklisterAccount = accounts[4]; const [minter, lostAndFound, alice, bob] = accounts.slice(9); before(async () => { @@ -46,6 +58,13 @@ contract("V2_2Upgrader", (accounts) => { }); await proxyAsV2_1.initializeV2("USD Coin"); await proxyAsV2_1.initializeV2_1(lostAndFound); + + // Initially blacklist all these accounts. + await Promise.all( + accountsToBlacklist.map((account) => + proxyAsV2_2.blacklist(account, { from: blacklisterAccount }) + ) + ); }); describe("proxy", () => { @@ -74,6 +93,15 @@ contract("V2_2Upgrader", (accounts) => { }); }); + describe("accountsToBlacklist", () => { + it("should return the correct list of addresses read in blacklist.test.js", async () => { + const actualAccountsToBlacklist = await v2_2Upgrader.accountsToBlacklist(); + expect(actualAccountsToBlacklist.map(toLower)).to.deep.equal( + accountsToBlacklist.map(toLower) + ); + }); + }); + describe("withdrawFiatToken", () => { it("should return the FiatToken to the transaction sender", async () => { // Mint 0.2 FiatToken. @@ -146,6 +174,12 @@ contract("V2_2Upgrader", (accounts) => { true ); + // All accountsToBlacklist are still blacklisted + const areAccountsBlacklisted = await Promise.all( + accountsToBlacklist.map((account) => proxyAsV2_2.isBlacklisted(account)) + ); + expect(areAccountsBlacklisted.every((b) => b)).to.be.true; + // mint works as expected await proxyAsV2_2.configureMinter(minter, 1000e6, { from: await proxyAsV2_2.masterMinter(), @@ -216,6 +250,7 @@ contract("V2_2Upgrader", (accounts) => { _fiatTokenProxy.address, _v1_1Implementation.address, // provide V1.1 implementation instead of V2.2 originalProxyAdmin, + [], { from: upgraderOwner } ); @@ -245,6 +280,124 @@ contract("V2_2Upgrader", (accounts) => { v2_1Implementation.address ); }); + + it("reverts if blacklisting an account that was not blacklisted", async () => { + const _fiatTokenProxy = await FiatTokenProxy.new( + v2_1Implementation.address, + { from: originalProxyAdmin } + ); + await initializeToVersion(_fiatTokenProxy, "2.1", minter, lostAndFound); + const _proxyAsV2_1 = await FiatTokenV2_1.at(_fiatTokenProxy.address); + + const _v2_2Implementation = await FiatTokenV2_2.new(); + const upgraderOwner = accounts[0]; + + // Try blacklisting an account that was not originally blacklisted. + const accountsToBlacklist = generateAccounts(1); + const _v2_2Upgrader = await V2_2Upgrader.new( + _fiatTokenProxy.address, + _v2_2Implementation.address, + originalProxyAdmin, + accountsToBlacklist, + { from: upgraderOwner } + ); + + // Transfer 0.2 FiatToken to the contract + await _proxyAsV2_1.configureMinter(minter, 2e5, { + from: await _proxyAsV2_1.masterMinter(), + }); + await _proxyAsV2_1.mint(minter, 2e5, { from: minter }); + await _proxyAsV2_1.transfer(_v2_2Upgrader.address, 2e5, { from: minter }); + + // Transfer admin role to the contract + await _fiatTokenProxy.changeAdmin(_v2_2Upgrader.address, { + from: originalProxyAdmin, + }); + + // Upgrade should fail because the account to blacklist was not previously blacklisted. + await expectRevert( + _v2_2Upgrader.upgrade({ from: upgraderOwner }), + "FiatTokenV2_2: Blacklisting previously unblacklisted account!" + ); + + // The proxy admin role is not transferred + expect(await _fiatTokenProxy.admin()).to.equal(_v2_2Upgrader.address); + + // The implementation is left unchanged + expect(await _fiatTokenProxy.implementation()).to.equal( + v2_1Implementation.address + ); + }); + + describe("gas tests", () => { + const gasTests: [number, number][] = [ + [100, BLOCK_GAS_LIMIT * 0.1], + [500, BLOCK_GAS_LIMIT * 0.5], + ]; + gasTests.forEach(([numAccounts, gasTarget]) => { + it(`should not exceed ${gasTarget} gas when blacklisting ${numAccounts} accounts`, async () => { + const accountsToBlacklist = generateAccounts(numAccounts); + + const _fiatTokenProxy = await FiatTokenProxy.new( + v2_1Implementation.address, + { from: originalProxyAdmin } + ); + await initializeToVersion( + _fiatTokenProxy, + "2.1", + minter, + lostAndFound + ); + const _proxyAsV2_1 = await FiatTokenV2_1.at(_fiatTokenProxy.address); + + // Blacklist the accounts in _deprecatedBlacklist first + await Promise.all( + accountsToBlacklist.map((a) => + _proxyAsV2_1.blacklist(a, { from: minter }) + ) + ); + + // Set up the V2_2Upgrader + const _v2_2Implementation = await FiatTokenV2_2.new(); + const upgraderOwner = accounts[0]; + const _v2_2Upgrader = await V2_2Upgrader.new( + _fiatTokenProxy.address, + _v2_2Implementation.address, + originalProxyAdmin, + accountsToBlacklist, + { from: upgraderOwner, gas: BLOCK_GAS_LIMIT } + ); + + // Transfer 0.2 FiatToken to the contract + await _proxyAsV2_1.configureMinter(minter, 2e5, { + from: await _proxyAsV2_1.masterMinter(), + }); + await _proxyAsV2_1.mint(minter, 2e5, { from: minter }); + await _proxyAsV2_1.transfer(_v2_2Upgrader.address, 2e5, { + from: minter, + }); + + // Transfer admin role to the contract + await _fiatTokenProxy.changeAdmin(_v2_2Upgrader.address, { + from: originalProxyAdmin, + }); + + // Perform the upgrade. + const txReceipt = await _v2_2Upgrader.upgrade({ + from: upgraderOwner, + gas: BLOCK_GAS_LIMIT, + }); + const gasUsed = txReceipt.receipt.gasUsed; + console.log({ numAccounts, gasUsed }); + expect(gasUsed).to.be.lessThan(gasTarget); + + // Sanity check that upgrade worked. + expect(await _fiatTokenProxy.implementation()).to.equal( + _v2_2Implementation.address + ); + }); + }); + }); }); describe("abortUpgrade", () => { @@ -260,6 +413,7 @@ contract("V2_2Upgrader", (accounts) => { _fiatTokenProxy.address, v2_1Implementation.address, originalProxyAdmin, + [], { from: upgraderOwner } ); const _v2_2UpgraderHelperAddress = await _v2_2Upgrader.helper(); diff --git a/utils.js b/utils.js new file mode 100644 index 000000000..81296d78a --- /dev/null +++ b/utils.js @@ -0,0 +1,52 @@ +/** + * SPDX-License-Identifier: MIT + * + * Copyright (c) 2018-2023 CENTRE SECZ + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + */ + +const _ = require("lodash"); +const fs = require("fs"); +const web3 = require("web3"); + +/** + * Helper function to read the blacklist file. + * @param blacklistFilePath {string} the filepath to the blacklist file. + * @returns {string[]} the list of addresses in the file. + */ +function readBlacklistFile(blacklistFilePath) { + if (!fs.existsSync(blacklistFilePath)) { + throw new Error(`'${blacklistFilePath}' does not exist!`); + } + let addresses = require(blacklistFilePath); + addresses = _.uniqBy(addresses, (a) => a.toLowerCase()); // Deduplicate any addresses in the file + + // Validate that addresses' integrity + for (const address of addresses) { + if (!web3.utils.isAddress(address)) { + throw new Error( + `Address '${address}' in '${blacklistFilePath}' is not valid address!` + ); + } + } + return addresses; +} + +module.exports = { readBlacklistFile };