From 24f9fcfdff4ef04fd47d459aaa88741c66c5dba4 Mon Sep 17 00:00:00 2001 From: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Date: Tue, 11 Jul 2023 11:30:31 +0200 Subject: [PATCH] Implement Gnosis withdrawals pull (#45) * Withdrawals pull * Update maxFailedWithdrawalsToProcess variable name * Delete settings.json * Place all variables at the top * Update executeSystemWithdrawals comment * chore: fix all solhint issues (#49) * chore: remove global imports * chore: ignore _deprecatedUnused * chore: re-add _deprecatedUnused comment * chore: fix system tx inputs (#50) * chore: add system tx with right signature * fix: make signature public and fix tests * Update SBCDepositContract comment * Update SBCDepositContract.sol --------- Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> --------- Co-authored-by: Philippe Schommers --- contracts/SBCDepositContract.sol | 190 ++++--------------- contracts/SBCDepositContractProxy.sol | 4 +- contracts/SBCToken.sol | 11 +- contracts/SBCTokenProxy.sol | 4 +- contracts/SBCWrapper.sol | 15 +- contracts/SBCWrapperProxy.sol | 6 +- contracts/interfaces/IERC677.sol | 2 +- contracts/interfaces/IPermittableToken.sol | 2 +- contracts/interfaces/IWithdrawalContract.sol | 30 --- contracts/test/SBCInit.sol | 12 +- contracts/test/UnsafeToken.sol | 5 +- contracts/test/UnsafeTokenProxy.sol | 10 +- contracts/utils/Claimable.sol | 2 +- contracts/utils/EIP1967Proxy.sol | 2 +- contracts/utils/PausableEIP1967Admin.sol | 4 +- package.json | 4 +- test/deposit.js | 181 +++--------------- yarn.lock | 60 +++--- 18 files changed, 147 insertions(+), 397 deletions(-) diff --git a/contracts/SBCDepositContract.sol b/contracts/SBCDepositContract.sol index ae6f5cc..cd0ee87 100644 --- a/contracts/SBCDepositContract.sol +++ b/contracts/SBCDepositContract.sol @@ -2,14 +2,15 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import "@openzeppelin/contracts/utils/introspection/IERC165.sol"; -import "./interfaces/IDepositContract.sol"; -import "./interfaces/IERC677Receiver.sol"; -import "./interfaces/IUnwrapper.sol"; -import "./interfaces/IWithdrawalContract.sol"; -import "./utils/PausableEIP1967Admin.sol"; -import "./utils/Claimable.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {IERC165} from "@openzeppelin/contracts/utils/introspection/IERC165.sol"; +import {IDepositContract} from "./interfaces/IDepositContract.sol"; +import {IERC677Receiver} from "./interfaces/IERC677Receiver.sol"; +import {IUnwrapper} from "./interfaces/IUnwrapper.sol"; +import {IWithdrawalContract} from "./interfaces/IWithdrawalContract.sol"; +import {PausableEIP1967Admin} from "./utils/PausableEIP1967Admin.sol"; +import {Claimable} from "./utils/Claimable.sol"; /** * @title SBCDepositContract @@ -39,6 +40,9 @@ contract SBCDepositContract is IERC20 public immutable stake_token; + address private constant SYSTEM_WITHDRAWAL_EXECUTOR = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; + mapping(address => uint256) public withdrawableAmount; + constructor(address _token) { stake_token = IERC20(_token); } @@ -230,182 +234,68 @@ contract SBCDepositContract is /*** Withdrawal part ***/ - address private constant SYSTEM_WITHDRAWAL_EXECUTOR = 0xffffFFFfFFffffffffffffffFfFFFfffFFFfFFfE; - - uint256 private constant DEFAULT_GAS_PER_WITHDRAWAL = 300000; - - /** - * @dev Function to be used to process a withdrawal. - * Actually it is an internal function, only this contract can call it. - * This is done in order to roll back all changes in case of revert. - * @param _amount Amount to be withdrawn. - * @param _receiver Receiver of the withdrawal. - */ - function processWithdrawalInternal(uint256 _amount, address _receiver) external { - require(msg.sender == address(this), "Should be used only as an internal call"); - stake_token.safeTransfer(_receiver, _amount); - } - - /** - * @dev Internal function to be used to process a withdrawal. - * Uses processWithdrawalInternal under the hood. - * Call to this function will revert only if it ran out of gas. - * @param _amount Amount to be withdrawn. - * @param _receiver Receiver of the withdrawal. - * @return success An indicator of whether the withdrawal was successful or not. - */ - function _processWithdrawal(uint256 _amount, address _receiver, uint256 gasLimit) internal returns (bool success) { - // Skip withdrawal that burns tokens to avoid a revert condition - // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/dad73159df3d3053c72b5e430fa8164330f18068/contracts/token/ERC20/ERC20.sol#L278 - if (_receiver == address(0)) { - return true; - } - - try this.processWithdrawalInternal{gas: gasLimit}(_amount, _receiver) { - return true; - } catch { - return false; - } - } - - struct FailedWithdrawalRecord { - uint256 amount; - address receiver; - uint64 withdrawalIndex; - } - mapping(uint256 => FailedWithdrawalRecord) public failedWithdrawals; - mapping(uint64 => uint256) public failedWithdrawalIndexByWithdrawalIndex; - uint256 public numberOfFailedWithdrawals; - uint64 public nextWithdrawalIndex; - /** - * @dev Function to be used to process a failed withdrawal (possibly partially). - * @param _failedWithdrawalId Id of a failed withdrawal. - * @param _amountToProceed Amount of token to withdraw (for the case it is impossible to withdraw the full amount) - * (available only for the receiver, will be ignored if other account tries to process the withdrawal). + * @dev Claim withdrawal amount for an address + * @param _address Address to transfer withdrawable tokens */ - function processFailedWithdrawal(uint256 _failedWithdrawalId, uint256 _amountToProceed) external whenNotPaused { - require(_failedWithdrawalId < numberOfFailedWithdrawals, "Failed withdrawal do not exist"); - - FailedWithdrawalRecord storage failedWithdrawalRecord = failedWithdrawals[_failedWithdrawalId]; - require(failedWithdrawalRecord.amount > 0, "Failed withdrawal already processed"); - - uint256 amountToProceed = failedWithdrawalRecord.amount; - if (_msgSender() == failedWithdrawalRecord.receiver) { - if (_amountToProceed != 0) { - require(_amountToProceed <= failedWithdrawalRecord.amount, "Invalid amount of tokens"); - amountToProceed = _amountToProceed; - } + function claimWithdrawal(address _address) public { + uint256 amount = withdrawableAmount[_address]; + if (amount > 0) { + withdrawableAmount[_address] = 0; + stake_token.safeTransfer(_address, amount); } - - failedWithdrawalRecord.amount -= amountToProceed; - bool success = _processWithdrawal(amountToProceed, failedWithdrawalRecord.receiver, gasleft()); - require(success, "Withdrawal processing failed"); - emit FailedWithdrawalProcessed(_failedWithdrawalId, amountToProceed, failedWithdrawalRecord.receiver); } - uint256 public failedWithdrawalsPointer; - /** - * @dev Function to be used to process failed withdrawals. - * Call to this function will revert only if it ran out of gas or it is a reentrant access to failed withdrawals processing. - * Call to this function doesn't transmit flow control to any untrusted contract and uses a constant gas limit for each withdrawal, - * so using constant gas limit and constant max number of withdrawals for calls of this function is ok. - * @param _maxNumberOfFailedWithdrawalsToProcess Maximum number of failed withdrawals to be processed. + * @dev Claim withdrawal amounts for an array of addresses + * @param _addresses Addresses to transfer withdrawable tokens */ - function processFailedWithdrawalsFromPointer(uint256 _maxNumberOfFailedWithdrawalsToProcess) public { - for (uint256 i = 0; i < _maxNumberOfFailedWithdrawalsToProcess; ++i) { - if (failedWithdrawalsPointer == numberOfFailedWithdrawals) { - break; - } - - FailedWithdrawalRecord storage failedWithdrawalRecord = failedWithdrawals[failedWithdrawalsPointer]; - if (failedWithdrawalRecord.amount > 0) { - // Reentrancy guard - uint256 amount = failedWithdrawalRecord.amount; - failedWithdrawalRecord.amount = 0; - - // Execute the withdrawal - bool success = _processWithdrawal(amount, failedWithdrawalRecord.receiver, DEFAULT_GAS_PER_WITHDRAWAL); - - if (!success) { - // Reset the record amount for the reentrancy guard - failedWithdrawalRecord.amount = amount; - break; - } - - emit FailedWithdrawalProcessed(failedWithdrawalsPointer, amount, failedWithdrawalRecord.receiver); - } - - ++failedWithdrawalsPointer; + function claimWithdrawals(address[] calldata _addresses) external { + for (uint256 i = 0; i < _addresses.length; ++i) { + claimWithdrawal(_addresses[i]); } } /** * @dev Function to be used only in the system transaction. - * Call to this function will revert only in three cases: + * Call to this function will revert only in case: * - the caller is not `SYSTEM_WITHDRAWAL_EXECUTOR` or `_admin()`; * - the length of `_amounts` array is not equal to the length of `_addresses` array; - * - it is a reentrant access to failed withdrawals processing; - * - the call ran out of gas. - * Call to this function doesn't transmit flow control to any untrusted contract and uses a constant gas limit for each withdrawal, - * so using constant gas limit and constant number of withdrawals (including failed withdrawals) for calls of this function is ok. - * @param _maxNumberOfFailedWithdrawalsToProcess Maximum number of failed withdrawals to be processed. + * Call to this function doesn't transmit flow control to any untrusted contract, nor does any operation of unbounded gas usage. + * NOTE: This function signature is hardcoded in the Gnosis execution layer clients. Changing this signature without updating the + * clients will cause block verification of any post-shangai block to fail. The function signature cannonical spec is here + * https://github.com/gnosischain/specs/blob/master/execution/withdrawals.md + * Note: chiado network requires this signature to sync post-shapella blocks. This function signature can only be deprecated after + * deprecating chiado network of full sync up to a pre-specified block. + * @custom:deprecatedparam _deprecatedUnused Previously `maxFailedWithdrawalsToProcess` currently deprecated and ignored * @param _amounts Array of amounts to be withdrawn. * @param _addresses Array of addresses that should receive the corresponding amount of tokens. */ function executeSystemWithdrawals( - uint256 _maxNumberOfFailedWithdrawalsToProcess, + uint256 /* _deprecatedUnused */, uint64[] calldata _amounts, address[] calldata _addresses - ) external { + ) public { require( _msgSender() == SYSTEM_WITHDRAWAL_EXECUTOR || _msgSender() == _admin(), "This function should be called only by SYSTEM_WITHDRAWAL_EXECUTOR or _admin()" ); assert(_amounts.length == _addresses.length); - processFailedWithdrawalsFromPointer(_maxNumberOfFailedWithdrawalsToProcess); - for (uint256 i = 0; i < _amounts.length; ++i) { // Divide stake amount by 32 (1 GNO for validating instead of the 32 ETH expected) uint256 amount = (uint256(_amounts[i]) * 1 gwei) / 32; - bool success = _processWithdrawal(amount, _addresses[i], DEFAULT_GAS_PER_WITHDRAWAL); - - if (success) { - emit WithdrawalExecuted(amount, _addresses[i]); - } else { - failedWithdrawals[numberOfFailedWithdrawals] = FailedWithdrawalRecord({ - amount: amount, - receiver: _addresses[i], - withdrawalIndex: nextWithdrawalIndex - }); - // Shift `failedWithdrawalIndex` by one to allow `isWithdrawalProcessed()` - // to detect successful withdrawals - failedWithdrawalIndexByWithdrawalIndex[nextWithdrawalIndex] = numberOfFailedWithdrawals + 1; - emit WithdrawalFailed(numberOfFailedWithdrawals, amount, _addresses[i]); - ++numberOfFailedWithdrawals; - } - - // First withdrawal is index 0 - nextWithdrawalIndex++; + withdrawableAmount[_addresses[i]] += amount; } } /** - * @dev Check if a block's withdrawal has been fully processed or not - * @param _withdrawalIndex EIP-4895 withdrawal.index property + * @dev Forwards compatible signature for `executeSystemWithdrawals` to support its future deprecation + * Clients must support and use the signature specified in the spec at: + * https://github.com/gnosischain/specs/blob/master/execution/withdrawals.md */ - function isWithdrawalProcessed(uint64 _withdrawalIndex) external view returns (bool) { - require(_withdrawalIndex < nextWithdrawalIndex, "withdrawal_index out-of-bounds"); - // Only failed withdrawals are registered into failedWithdrawalByIndex, so successful withdrawals - // `_withdrawalIndex` return `failedWithdrawalIndex` 0. - uint256 failedWithdrawalIndex = failedWithdrawalIndexByWithdrawalIndex[_withdrawalIndex]; - if (failedWithdrawalIndex == 0) { - return true; - } - // `failedWithdrawalIndex` are shifted by one for the above case - return failedWithdrawals[failedWithdrawalIndex - 1].amount == 0; + function executeSystemWithdrawals(uint64[] calldata _amounts, address[] calldata _addresses) external { + executeSystemWithdrawals(0, _amounts, _addresses); } /** diff --git a/contracts/SBCDepositContractProxy.sol b/contracts/SBCDepositContractProxy.sol index 219218f..b8793c5 100644 --- a/contracts/SBCDepositContractProxy.sol +++ b/contracts/SBCDepositContractProxy.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.9; -import "./utils/EIP1967Proxy.sol"; -import "./SBCDepositContract.sol"; +import {EIP1967Proxy} from "./utils/EIP1967Proxy.sol"; +import {SBCDepositContract} from "./SBCDepositContract.sol"; /** * @title SBCDepositContractProxy diff --git a/contracts/SBCToken.sol b/contracts/SBCToken.sol index 78fc1af..741976e 100644 --- a/contracts/SBCToken.sol +++ b/contracts/SBCToken.sol @@ -2,11 +2,12 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol"; -import "./utils/Claimable.sol"; -import "./utils/PausableEIP1967Admin.sol"; -import "./interfaces/IERC677.sol"; -import "./interfaces/IERC677Receiver.sol"; +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {ERC20Pausable} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol"; +import {Claimable} from "./utils/Claimable.sol"; +import {PausableEIP1967Admin} from "./utils/PausableEIP1967Admin.sol"; +import {IERC677} from "./interfaces/IERC677.sol"; +import {IERC677Receiver} from "./interfaces/IERC677Receiver.sol"; /** * @title SBCToken diff --git a/contracts/SBCTokenProxy.sol b/contracts/SBCTokenProxy.sol index 1042093..2c40227 100644 --- a/contracts/SBCTokenProxy.sol +++ b/contracts/SBCTokenProxy.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.9; -import "./utils/EIP1967Proxy.sol"; -import "./SBCToken.sol"; +import {EIP1967Proxy} from "./utils/EIP1967Proxy.sol"; +import {SBCToken} from "./SBCToken.sol"; /** * @title SBCTokenProxy diff --git a/contracts/SBCWrapper.sol b/contracts/SBCWrapper.sol index 6d161b0..c33d1de 100644 --- a/contracts/SBCWrapper.sol +++ b/contracts/SBCWrapper.sol @@ -2,12 +2,15 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; -import "./interfaces/IUnwrapper.sol"; -import "./utils/PausableEIP1967Admin.sol"; -import "./SBCToken.sol"; -import "./SBCDepositContract.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol"; +import {IUnwrapper} from "./interfaces/IUnwrapper.sol"; +import {IERC677Receiver} from "./interfaces/IERC677Receiver.sol"; +import {PausableEIP1967Admin} from "./utils/PausableEIP1967Admin.sol"; +import {SBCToken} from "./SBCToken.sol"; +import {SBCDepositContract} from "./SBCDepositContract.sol"; +import {Claimable} from "./utils/Claimable.sol"; /** * @title SBCWrapper diff --git a/contracts/SBCWrapperProxy.sol b/contracts/SBCWrapperProxy.sol index a28e178..b99e0d3 100644 --- a/contracts/SBCWrapperProxy.sol +++ b/contracts/SBCWrapperProxy.sol @@ -2,8 +2,10 @@ pragma solidity 0.8.9; -import "./utils/EIP1967Proxy.sol"; -import "./SBCWrapper.sol"; +import {EIP1967Proxy} from "./utils/EIP1967Proxy.sol"; +import {SBCWrapper} from "./SBCWrapper.sol"; +import {SBCToken} from "./SBCToken.sol"; +import {SBCDepositContract} from "./SBCDepositContract.sol"; /** * @title SBCWrapperProxy diff --git a/contracts/interfaces/IERC677.sol b/contracts/interfaces/IERC677.sol index 2aaee04..3bafdb5 100644 --- a/contracts/interfaces/IERC677.sol +++ b/contracts/interfaces/IERC677.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; interface IERC677 is IERC20 { function transferAndCall(address to, uint256 amount, bytes calldata data) external; diff --git a/contracts/interfaces/IPermittableToken.sol b/contracts/interfaces/IPermittableToken.sol index d53c88c..446b01c 100644 --- a/contracts/interfaces/IPermittableToken.sol +++ b/contracts/interfaces/IPermittableToken.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.9; -import "./IERC677.sol"; +import {IERC677} from "./IERC677.sol"; interface IPermittableToken is IERC677 { function permit( diff --git a/contracts/interfaces/IWithdrawalContract.sol b/contracts/interfaces/IWithdrawalContract.sol index fb04b23..4d2c51a 100644 --- a/contracts/interfaces/IWithdrawalContract.sol +++ b/contracts/interfaces/IWithdrawalContract.sol @@ -21,34 +21,4 @@ interface IWithdrawalContract { uint64[] calldata _amounts, address[] calldata _addresses ) external; - - /// @notice Executed withdrawal event. - event WithdrawalExecuted(uint256 _amount, address indexed _address); - - /// @notice Failed withdrawal event. - event WithdrawalFailed(uint256 indexed _failedWithdrawalId, uint256 _amount, address indexed _address); - - /** - * @dev Function to be used to process failed withdrawals. - * Call to this function will revert only if it ran out of gas or it is a reentrant access to failed withdrawals processing. - * Call to this function doesn't transmit flow control to any untrusted contract and uses a constant gas limit for each withdrawal, - * so using constant gas limit and constant max number of withdrawals for calls of this function is ok. - * @param _maxNumberOfFailedWithdrawalsToProcess Maximum number of failed withdrawals to be processed. - */ - function processFailedWithdrawalsFromPointer(uint256 _maxNumberOfFailedWithdrawalsToProcess) external; - - /** - * @dev Function to be used to process a failed withdrawal (possibly partially). - * @param _failedWithdrawalId Id of a failed withdrawal. - * @param _amountToProceed Amount of token to withdraw (for the case it is impossible to withdraw the full amount) - * (available only for the receiver, will be ignored if other account tries to process the withdrawal). - */ - function processFailedWithdrawal(uint256 _failedWithdrawalId, uint256 _amountToProceed) external; - - /// @notice Processed (possibly partially) failed withdrawal event. - event FailedWithdrawalProcessed( - uint256 indexed _failedWithdrawalId, - uint256 _processedAmount, - address indexed _address - ); } diff --git a/contracts/test/SBCInit.sol b/contracts/test/SBCInit.sol index 5db44a3..a795d2f 100644 --- a/contracts/test/SBCInit.sol +++ b/contracts/test/SBCInit.sol @@ -2,12 +2,12 @@ pragma solidity ^0.8.9; -import "../SBCDepositContractProxy.sol"; -import "../SBCToken.sol"; -import "../SBCTokenProxy.sol"; -import "../SBCWrapper.sol"; -import "../SBCWrapperProxy.sol"; -import "./UnsafeToken.sol"; +import {SBCDepositContractProxy} from "../SBCDepositContractProxy.sol"; +import {SBCToken} from "../SBCToken.sol"; +import {SBCTokenProxy} from "../SBCTokenProxy.sol"; +import {SBCWrapper} from "../SBCWrapper.sol"; +import {SBCWrapperProxy} from "../SBCWrapperProxy.sol"; +import {UnsafeToken} from "./UnsafeToken.sol"; contract SBCInit { constructor( diff --git a/contracts/test/UnsafeToken.sol b/contracts/test/UnsafeToken.sol index ed5e22b..823a437 100644 --- a/contracts/test/UnsafeToken.sol +++ b/contracts/test/UnsafeToken.sol @@ -2,8 +2,9 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol"; -import "../utils/PausableEIP1967Admin.sol"; +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {ERC20Pausable} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20Pausable.sol"; +import {PausableEIP1967Admin} from "../utils/PausableEIP1967Admin.sol"; /** * @title Unsafe ERC20 that allows admin to mint or steal tokens diff --git a/contracts/test/UnsafeTokenProxy.sol b/contracts/test/UnsafeTokenProxy.sol index 682ba89..e1281dd 100644 --- a/contracts/test/UnsafeTokenProxy.sol +++ b/contracts/test/UnsafeTokenProxy.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.9; -import "../utils/EIP1967Proxy.sol"; -import "./UnsafeToken.sol"; +import {EIP1967Proxy} from "../utils/EIP1967Proxy.sol"; +import {UnsafeToken} from "./UnsafeToken.sol"; /** * @title UnsafeTokenProxy @@ -16,11 +16,7 @@ contract UnsafeTokenProxy is EIP1967Proxy { string private _name; string private _symbol; - constructor( - address _admin, - string memory name, - string memory symbol - ) { + constructor(address _admin, string memory name, string memory symbol) { _setAdmin(_admin); _setImplementation(address(new UnsafeToken())); _name = name; diff --git a/contracts/utils/Claimable.sol b/contracts/utils/Claimable.sol index 4d78622..72ea17d 100644 --- a/contracts/utils/Claimable.sol +++ b/contracts/utils/Claimable.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; /** * @title Claimable diff --git a/contracts/utils/EIP1967Proxy.sol b/contracts/utils/EIP1967Proxy.sol index 8d99f85..3ccd23a 100644 --- a/contracts/utils/EIP1967Proxy.sol +++ b/contracts/utils/EIP1967Proxy.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.9; -import "./EIP1967Admin.sol"; +import {EIP1967Admin} from "./EIP1967Admin.sol"; /** * @title EIP1967Proxy diff --git a/contracts/utils/PausableEIP1967Admin.sol b/contracts/utils/PausableEIP1967Admin.sol index e693d03..fd1af98 100644 --- a/contracts/utils/PausableEIP1967Admin.sol +++ b/contracts/utils/PausableEIP1967Admin.sol @@ -2,8 +2,8 @@ pragma solidity 0.8.9; -import "@openzeppelin/contracts/security/Pausable.sol"; -import "./EIP1967Admin.sol"; +import {Pausable} from "@openzeppelin/contracts/security/Pausable.sol"; +import {EIP1967Admin} from "./EIP1967Admin.sol"; /** * @title PausableEIP1967Admin diff --git a/package.json b/package.json index 78aca8c..22213e3 100644 --- a/package.json +++ b/package.json @@ -30,8 +30,8 @@ "chai-as-promised": "^7.1.1", "ethereumjs-util": "^7.1.2", "prettier": "^2.3.2", - "prettier-plugin-solidity": "^1.0.0-beta.17", - "solhint": "^3.3.6", + "prettier-plugin-solidity": "^1.1.3", + "solhint": "^3.4.1", "solhint-plugin-prettier": "^0.0.5", "solidity-coverage": "^0.7.17" } diff --git a/test/deposit.js b/test/deposit.js index b4177ea..fcf331c 100644 --- a/test/deposit.js +++ b/test/deposit.js @@ -205,93 +205,46 @@ contract('SBCDepositContractProxy', (accounts) => { const amounts = ['0x0000000000000000', '0x0000000000000000'] const elongatedAmounts = ['0x0000000000000000', '0x0000000000000000', '0x0000000000000000'] const addresses = [accounts[0], accounts[0]] - await contract.executeSystemWithdrawals(10, amounts, addresses, { from: accounts[1] }).should.be.rejected - await contract.executeSystemWithdrawals(10, elongatedAmounts, addresses, { from: accounts[0] }).should.be.rejected - await contract.executeSystemWithdrawals(10, amounts, addresses, { from: accounts[0] }) + await contract.executeSystemWithdrawals(amounts, addresses, { from: accounts[1] }).should.be.rejected + await contract.executeSystemWithdrawals(elongatedAmounts, addresses, { from: accounts[0] }).should.be.rejected + await contract.executeSystemWithdrawals(amounts, addresses, { from: accounts[0] }) }) - it('should correctly withdraw GNO, even with failed withdrawal', async () => { + it('should correctly withdraw GNO for one address', async () => { const amounts = ['0x0000000773594000'] // 32 * 10^9 const addresses = [accounts[1]] // simple withdrawal await stake.transfer(contract.address, depositAmount) - await contract.executeSystemWithdrawals(0, amounts, addresses) - const mGNOBalanceAfterFirstWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterFirstWithdrawal).to.be.equal(depositAmount) + await contract.executeSystemWithdrawals(amounts, addresses) + const claimableGNO = (await contract.withdrawableAmount(accounts[1])).toString() + expect(claimableGNO).to.be.equal(depositAmount) + await contract.claimWithdrawal(accounts[1]) + const GNOBalanceAfterWithdrawal = (await stake.balanceOf(accounts[1])).toString() + expect(GNOBalanceAfterWithdrawal).to.be.equal(depositAmount) + }) - // failed and processed by queue - await contract.executeSystemWithdrawals(0, amounts, addresses) - let numberOfFailedWithdrawals = (await contract.numberOfFailedWithdrawals()).toString() - expect(numberOfFailedWithdrawals).to.be.equal('1') - - await stake.transfer(contract.address, depositAmount) - - await contract.processFailedWithdrawalsFromPointer(5) - const mGNOBalanceAfterSecondWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterSecondWithdrawal).to.be.equal(web3.utils.toWei('2')) - let failedWithdrawalsPointer = (await contract.failedWithdrawalsPointer()).toString() - expect(failedWithdrawalsPointer).to.be.equal('1') - await contract.processFailedWithdrawal(0, 0).should.be.rejected - - - // failed and processed by queue in executeSystemWithdrawals - await contract.executeSystemWithdrawals(0, amounts, addresses) - numberOfFailedWithdrawals = (await contract.numberOfFailedWithdrawals()).toString() - expect(numberOfFailedWithdrawals).to.be.equal('2') - - await stake.transfer(contract.address, depositAmount) - - await contract.executeSystemWithdrawals(2, [], []) - const mGNOBalanceAfterThirdWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterThirdWithdrawal).to.be.equal(web3.utils.toWei('3')) - failedWithdrawalsPointer = (await contract.failedWithdrawalsPointer()).toString() - expect(failedWithdrawalsPointer).to.be.equal('2') - await contract.processFailedWithdrawal(1, 0).should.be.rejected - - - // failed and processed manually - await contract.executeSystemWithdrawals(0, amounts, addresses) - numberOfFailedWithdrawals = (await contract.numberOfFailedWithdrawals()).toString() - expect(numberOfFailedWithdrawals).to.be.equal('3') - - await stake.transfer(contract.address, depositAmount) - - await contract.processFailedWithdrawal(2, 0) - await contract.processFailedWithdrawal(2, 0).should.be.rejected - - let mGNOBalanceAfterFourthWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterFourthWithdrawal).to.be.equal(web3.utils.toWei('4')) - failedWithdrawalsPointer = (await contract.failedWithdrawalsPointer()).toString() - expect(failedWithdrawalsPointer).to.be.equal('2') - await contract.processFailedWithdrawalsFromPointer(5) - mGNOBalanceAfterFourthWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterFourthWithdrawal).to.be.equal(web3.utils.toWei('4')) - failedWithdrawalsPointer = (await contract.failedWithdrawalsPointer()).toString() - expect(failedWithdrawalsPointer).to.be.equal('3') - - - // failed and processed partially manually - await contract.executeSystemWithdrawals(0, amounts, addresses) - numberOfFailedWithdrawals = (await contract.numberOfFailedWithdrawals()).toString() - expect(numberOfFailedWithdrawals).to.be.equal('4') - - await stake.transfer(contract.address, depositAmount) - - await contract.processFailedWithdrawal(3, halfDepositAmount, { from : addresses[0] }) - - let mGNOBalanceAfterFifthWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterFifthWithdrawal).to.be.equal(web3.utils.toWei('4.5')) + it('should correctly withdraw GNO for multiple addresses', async () => { + const addressCount = 4; + const amounts = Array.from({length: addressCount}, () => '0x0000000773594000') // 32 * 10^9 + const addresses = accounts.slice(1, 1 + addressCount) - await contract.processFailedWithdrawal(3, 0) + // simple withdrawal + await stake.transfer(contract.address, web3.utils.toWei(String(addressCount))) - mGNOBalanceAfterFifthWithdrawal = (await stake.balanceOf(accounts[1])).toString() - expect(mGNOBalanceAfterFifthWithdrawal).to.be.equal(web3.utils.toWei('5')) + await contract.executeSystemWithdrawals(amounts, addresses) + for (let i = 0; i < addressCount; i++) { + const claimableGNO = (await contract.withdrawableAmount(addresses[i])).toString() + expect(claimableGNO).to.be.equal(depositAmount) + } - await contract.processFailedWithdrawal(3, 0).should.be.rejected - await contract.processFailedWithdrawal(4, 0).should.be.rejected + await contract.claimWithdrawals(addresses) + for (let i = 0; i < addressCount; i++) { + const GNOBalanceAfterWithdrawal = (await stake.balanceOf(addresses[i])).toString() + expect(GNOBalanceAfterWithdrawal).to.be.equal(depositAmount) + } }) it('should correctly withdraw mGNO with amount = 0', async () => { @@ -301,7 +254,7 @@ contract('SBCDepositContractProxy', (accounts) => { // simple withdrawal await token.transfer(contract.address, thirtyTwoEther) - assertSuccessfulWithdrawal(await contract.executeSystemWithdrawals(0, amounts, addresses)) + await contract.executeSystemWithdrawals(amounts, addresses) const mGNOBalanceAfterWithdrawal = (await token.balanceOf(accounts[1])).toString() expect(mGNOBalanceAfterWithdrawal).to.be.equal(web3.utils.toWei('0')) }) @@ -313,51 +266,11 @@ contract('SBCDepositContractProxy', (accounts) => { // simple withdrawal await token.transfer(contract.address, thirtyTwoEther) - assertSuccessfulWithdrawal(await contract.executeSystemWithdrawals(0, amounts, addresses)) + await contract.executeSystemWithdrawals(amounts, addresses) const mGNOBalanceAfterFirstWithdrawal = (await token.balanceOf(zeroAddress)).toString() expect(mGNOBalanceAfterFirstWithdrawal).to.be.equal(zeroEther) }) - it("isWithdrawalProcessed first withdrawal successful", async () => { - // No withdrawals yet, none should be processed - // Withdrawals state = [] - await assertWithdrawalsState([]); - - await executeSuccessfulWithdrawal(); - // Withdrawal state = [success] - await assertWithdrawalsState([true]); - - await executeFailedWithdrawal(); - // Withdrawal state = [success, failed] - await assertWithdrawalsState([true, false]); - - await reexecuteFailedWithdrawal(); - // Withdrawal state = [success, success] - await assertWithdrawalsState([true, true]); - }); - - it("isWithdrawalProcessed first withdrawal failed", async () => { - // No withdrawals yet, none should be processed - // Withdrawals state = [] - await assertWithdrawalsState([]); - - await executeFailedWithdrawal(); - // Withdrawal state = [failed] - await assertWithdrawalsState([false]); - - await executeFailedWithdrawal(); - // Withdrawal state = [failed, failed] - await assertWithdrawalsState([false, false]); - - await reexecuteFailedWithdrawal(); - // Withdrawal state = [failed, success] - await assertWithdrawalsState([true, false]); - - await reexecuteFailedWithdrawal(); - // Withdrawal state = [success, success] - await assertWithdrawalsState([true, true]); - }); - it('should claim tokens', async () => { const otherToken = await IERC677.new() await stake.transfer(contract.address, 1) @@ -388,40 +301,4 @@ contract('SBCDepositContractProxy', (accounts) => { await contract.unwrapTokens(wrapper.address, token.address) expect((await stake.balanceOf(contract.address)).toString()).to.be.equal(web3.utils.toWei('42')) }) - - async function executeSuccessfulWithdrawal() { - const amounts = ["0x0000000773594000"] // 32 * 10^9 - const addresses = [accounts[1]] - await stake.transfer(contract.address, depositAmount) - await contract.executeSystemWithdrawals(0, amounts, addresses) - } - - async function executeFailedWithdrawal() { - const amounts = ["0x0000000773594000"] // 32 * 10^9 - const addresses = [accounts[1]] - await contract.executeSystemWithdrawals(0, amounts, addresses) - } - - async function reexecuteFailedWithdrawal() { - await stake.transfer(contract.address, depositAmount) - await contract.executeSystemWithdrawals(1, [], []) - } - - // Call with bool array where true = successful withdrawal - async function assertWithdrawalsState(withdrawalState) { - for (let i = 0; i < withdrawalState.length; i++) { - expect(await contract.isWithdrawalProcessed(i)).equal( - withdrawalState[i], - `wrong isWithdrawalProcessed(${i}), withdrawalState ${JSON.stringify(withdrawalState)}` - ) - } - // Should revert with out-of-bounds on the next withdrawalState - await contract.isWithdrawalProcessed(withdrawalState.length).should.be.rejected - } }) - -function assertSuccessfulWithdrawal(tx) { - const withdrawEvent = tx.logs.find(log => log.event.startsWith("Withdrawal")) - if (!withdrawEvent) throw Error('tx has no Withdraw* events') - expect(withdrawEvent.event).equal('WithdrawalExecuted') -} \ No newline at end of file diff --git a/yarn.lock b/yarn.lock index 9d9116c..d592f14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -111,11 +111,11 @@ xss "^1.0.8" "@babel/code-frame@^7.0.0": - version "7.21.4" - resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.21.4.tgz#d0fa9e4413aca81f2b23b9442797bda1826edb39" - integrity sha512-LYvhNKfwWSPpocw8GI7gpK2nq3HSDuEPC/uSYaALSJu9xjsalaaYFOq0Pwt5KmVqwEbZlDu81aLXwBOmD/Fv9g== + version "7.22.5" + resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.22.5.tgz#234d98e1551960604f1246e6475891a570ad5658" + integrity sha512-Xmwn266vad+6DAqEB2A6V/CcZVp62BbwVmcOJc2RPuwih1kw02TjQvWVWlcKGbBPd+8/0V5DEkOcizRGYsspYQ== dependencies: - "@babel/highlight" "^7.18.6" + "@babel/highlight" "^7.22.5" "@babel/compat-data@^7.17.7", "@babel/compat-data@^7.21.4": version "7.21.4" @@ -162,22 +162,27 @@ resolved "https://registry.yarnpkg.com/@babel/helper-string-parser/-/helper-string-parser-7.19.4.tgz#38d3acb654b4701a9b77fb0615a96f775c3a9e63" integrity sha512-nHtDoQcuqFmwYNYPz3Rah5ph2p8PFeFCsZk9A/48dPc/rGocJ5J3hAAZ7pb76VWX3fZKu+uEr/FhH5jLx7umrw== -"@babel/helper-validator-identifier@^7.18.6", "@babel/helper-validator-identifier@^7.19.1": +"@babel/helper-validator-identifier@^7.19.1": version "7.19.1" resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.19.1.tgz#7eea834cf32901ffdc1a7ee555e2f9c27e249ca2" integrity sha512-awrNfaMtnHUr653GgGEs++LlAvW6w+DcPrOliSMXWCKo597CwL5Acf/wWdNkf/tfEQE3mjkeD1YOVZOUV/od1w== +"@babel/helper-validator-identifier@^7.22.5": + version "7.22.5" + resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.22.5.tgz#9544ef6a33999343c8740fa51350f30eeaaaf193" + integrity sha512-aJXu+6lErq8ltp+JhkJUfk1MTGyuA4v7f3pA+BJ5HLfNC6nAQ0Cpi9uOquUj8Hehg0aUiHzWQbOVJGao6ztBAQ== + "@babel/helper-validator-option@^7.21.0": version "7.21.0" resolved "https://registry.yarnpkg.com/@babel/helper-validator-option/-/helper-validator-option-7.21.0.tgz#8224c7e13ace4bafdc4004da2cf064ef42673180" integrity sha512-rmL/B8/f0mKS2baE9ZpyTcTavvEuWhTTW8amjzXNvYG4AwBsqTLikfXsEofsJEfKHf+HQVQbFOHy6o+4cnC/fQ== -"@babel/highlight@^7.18.6": - version "7.18.6" - resolved "https://registry.yarnpkg.com/@babel/highlight/-/highlight-7.18.6.tgz#81158601e93e2563795adcbfbdf5d64be3f2ecdf" - integrity sha512-u7stbOuYjaPezCuLj29hNW1v64M2Md2qupEKP1fHc7WdOA3DgLh37suiSrZYY7haUB7iBeQZ9P1uiRF359do3g== +"@babel/highlight@^7.22.5": + version "7.22.5" + resolved "https://registry.yarnpkg.com/@babel/highlight/-/highlight-7.22.5.tgz#aa6c05c5407a67ebce408162b7ede789b4d22031" + integrity sha512-BSKlD1hgnedS5XRnGOljZawtag7H1yPfQp0tdNJCHoH6AZ+Pcm9VvkrK59/Yy593Ypg0zMxH2BxD1VPYUQ7UIw== dependencies: - "@babel/helper-validator-identifier" "^7.18.6" + "@babel/helper-validator-identifier" "^7.22.5" chalk "^2.0.0" js-tokens "^4.0.0" @@ -1328,9 +1333,9 @@ ansi-styles@^4.0.0, ansi-styles@^4.1.0: color-convert "^2.0.1" antlr4@^4.11.0: - version "4.12.0" - resolved "https://registry.yarnpkg.com/antlr4/-/antlr4-4.12.0.tgz#e2323fbb057c77068a174914b0533398aeaba56a" - integrity sha512-23iB5IzXJZRZeK9TigzUyrNc9pSmNqAerJRBcNq1ETrmttMWRgaYZzC561IgEO3ygKsDJTYDTozABXa4b/fTQQ== + version "4.13.0" + resolved "https://registry.yarnpkg.com/antlr4/-/antlr4-4.13.0.tgz#25c0b17f0d9216de114303d38bafd6f181d5447f" + integrity sha512-zooUbt+UscjnWyOrsuY/tVFL4rwrAGwOivpQmvmUDE22hy/lUA467Rc1rcixyRwcRUIXFYBwv7+dClDSHdmmew== antlr4ts@^0.5.0-alpha.4: version "0.5.0-alpha.4" @@ -2159,9 +2164,9 @@ combined-stream@^1.0.6, combined-stream@^1.0.8, combined-stream@~1.0.6: delayed-stream "~1.0.0" commander@^10.0.0: - version "10.0.0" - resolved "https://registry.yarnpkg.com/commander/-/commander-10.0.0.tgz#71797971162cd3cf65f0b9d24eb28f8d303acdf1" - integrity sha512-zS5PnTI22FIRM6ylNW8G4Ap0IEOyk62fhLSD0+uHRT9McRCLGpkVNvao4bjimpK/GShynyQkFFxHhwMcETmduA== + version "10.0.1" + resolved "https://registry.yarnpkg.com/commander/-/commander-10.0.1.tgz#881ee46b4f77d1c1dccc5823433aa39b022cbe06" + integrity sha512-y4Mg2tXshplEbSGzx7amzPwKKOCGuoSRP/CjEdwwk0FOGlUbq6lKuoyDZTNZkmxHdJtp54hdfY/JUrdL7Xfdug== commander@^2.20.3: version "2.20.3" @@ -2259,9 +2264,9 @@ cors@^2.8.1, cors@^2.8.5: vary "^1" cosmiconfig@^8.0.0: - version "8.1.3" - resolved "https://registry.yarnpkg.com/cosmiconfig/-/cosmiconfig-8.1.3.tgz#0e614a118fcc2d9e5afc2f87d53cd09931015689" - integrity sha512-/UkO2JKI18b5jVMJUp0lvKFMpa/Gye+ZgZjKD+DGEN9y7NRcf/nK1A0sp67ONmKtnDCNMS44E6jrk0Yc3bDuUw== + version "8.2.0" + resolved "https://registry.yarnpkg.com/cosmiconfig/-/cosmiconfig-8.2.0.tgz#f7d17c56a590856cd1e7cee98734dca272b0d8fd" + integrity sha512-3rTMnFJA1tCOPwRxtgF4wd7Ab2qvDbL8jX+3smjIbS4HlZBagTlpERbdN7iAbWlrfxE3M8c27kTwTawQ7st+OQ== dependencies: import-fresh "^3.2.1" js-yaml "^4.1.0" @@ -3126,9 +3131,9 @@ fast-deep-equal@^3.1.1: integrity sha512-f3qQ9oQy9j2AhBe/H9VC91wLmKBCCU/gDOnKNAYG5hswO7BLKj09Hc5HYNz9cGI++xlpDCIgDaitVs03ATR84Q== fast-diff@^1.1.2, fast-diff@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/fast-diff/-/fast-diff-1.2.0.tgz#73ee11982d86caaf7959828d519cfe927fac5f03" - integrity sha512-xJuoT5+L99XlZ8twedaRf6Ax2TgQVxvgZOYoPKqZufmJib0tL2tegPBOZb1pVNgIhlqDlA0eO0c3wBvQcmzx4w== + version "1.3.0" + resolved "https://registry.yarnpkg.com/fast-diff/-/fast-diff-1.3.0.tgz#ece407fa550a64d638536cd727e129c61616e0f0" + integrity sha512-VxPP4NqbUjj6MaAOafWeUn2cXWLcCtljklUtZf0Ind4XQ+QPtmA0b18zZy0jIQx+ExRVCR/ZQpBmik5lXshNsw== fast-glob@^3.0.3: version "3.2.12" @@ -5350,7 +5355,7 @@ prettier-linter-helpers@^1.0.0: dependencies: fast-diff "^1.1.2" -prettier-plugin-solidity@^1.0.0-beta.17: +prettier-plugin-solidity@^1.1.3: version "1.1.3" resolved "https://registry.yarnpkg.com/prettier-plugin-solidity/-/prettier-plugin-solidity-1.1.3.tgz#9a35124f578404caf617634a8cab80862d726cba" integrity sha512-fQ9yucPi2sBbA2U2Xjh6m4isUTJ7S7QLc/XDDsktqqxYfTwdYKJ0EnnywXHwCGAaYbQNK+HIYPL1OemxuMsgeg== @@ -5359,11 +5364,16 @@ prettier-plugin-solidity@^1.0.0-beta.17: semver "^7.3.8" solidity-comments-extractor "^0.0.7" -prettier@^2.3.2, prettier@^2.8.3: +prettier@^2.3.2: version "2.8.7" resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.7.tgz#bb79fc8729308549d28fe3a98fce73d2c0656450" integrity sha512-yPngTo3aXUUmyuTjeTUT75txrf+aMh9FiD7q9ZE/i6r0bPb22g4FsE6Y338PQX1bmfy08i9QQCB7/rcUAVntfw== +prettier@^2.8.3: + version "2.8.8" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.8.tgz#e8c5d7e98a4305ffe3de2e1fc4aca1a71c28b1da" + integrity sha512-tdN8qQGvNjw4CHbY+XXk0JgCXn9QiF21a55rBe5LJAU+kDyC4WQn4+awm2Xfk2lQMk5fKup9XgzTZtGkjBdP9Q== + process-nextick-args@~2.0.0: version "2.0.1" resolved "https://registry.yarnpkg.com/process-nextick-args/-/process-nextick-args-2.0.1.tgz#7820d9b16120cc55ca9ae7792680ae7dba6d7fe2" @@ -5994,7 +6004,7 @@ solhint-plugin-prettier@^0.0.5: dependencies: prettier-linter-helpers "^1.0.0" -solhint@^3.3.6: +solhint@^3.4.1: version "3.4.1" resolved "https://registry.yarnpkg.com/solhint/-/solhint-3.4.1.tgz#8ea15b21c13d1be0b53fd46d605a24d0b36a0c46" integrity sha512-pzZn2RlZhws1XwvLPVSsxfHrwsteFf5eySOhpAytzXwKQYbTCJV6z8EevYDiSVKMpWrvbKpEtJ055CuEmzp4Xg==