refactor!: remove withdrawERC20Tokens function

cartesi · Dec 4, 2023 · e2bc71c · e2bc71c
1 parent fd3c717
commit e2bc71c
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 138 deletions.
diff --git a/onchain/rollups/.changeset/rotten-yaks-help.md b/onchain/rollups/.changeset/rotten-yaks-help.md
@@ -0,0 +1,6 @@
+---
+"@cartesi/rollups": major
+---
+
+Removed `AuthorityWithdrawalFailed` error from the `Authority` contract.
+This error was removed because it would only be raised by the `withdrawERC20Tokens` function, which was removed.
diff --git a/onchain/rollups/.changeset/short-bears-flash.md b/onchain/rollups/.changeset/short-bears-flash.md
@@ -0,0 +1,8 @@
+---
+"@cartesi/rollups": major
+---
+
+Removed the `withdrawERC20Tokens` function from `Authority` contract.
+This function was removed due to the lack of usage, and because implementing a similar function for `Quorum` would not be possible with `@openzeppelin/[email protected]`.
+Users should not transfer ERC-20 tokens to `Authority` contracts, as it now no longer defines an entry point for withdrawing them, leaving them stuck there forever.
+Users should not try to call this function, as it is no longer present in the contract.
diff --git a/onchain/rollups/contracts/consensus/authority/Authority.sol b/onchain/rollups/contracts/consensus/authority/Authority.sol
@@ -4,7 +4,6 @@
 pragma solidity ^0.8.8;
 
 import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
-import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 
 import {IConsensus} from "../IConsensus.sol";
 import {AbstractConsensus} from "../AbstractConsensus.sol";
@@ -25,9 +24,6 @@ contract Authority is AbstractConsensus, Ownable {
     /// @dev MUST be triggered on a successful call to `setHistory`.
     event NewHistory(IHistory history);
 
-    /// @notice Raised when a transfer of tokens from an authority to a recipient fails.
-    error AuthorityWithdrawalFailed();
-
     /// @notice Constructs an `Authority` contract.
     /// @param _owner The initial contract owner
     constructor(address _owner) {
@@ -83,21 +79,4 @@ contract Authority is AbstractConsensus, Ownable {
     ) external view override returns (bytes32, uint256, uint256) {
         return history.getClaim(_dapp, _proofContext);
     }
-
-    /// @notice Transfer some amount of ERC-20 tokens to a recipient.
-    /// @param _token The token contract
-    /// @param _recipient The recipient address
-    /// @param _amount The amount of tokens to be withdrawn
-    /// @dev Can only be called by the `Authority` owner.
-    function withdrawERC20Tokens(
-        IERC20 _token,
-        address _recipient,
-        uint256 _amount
-    ) external onlyOwner {
-        bool success = _token.transfer(_recipient, _amount);
-
-        if (!success) {
-            revert AuthorityWithdrawalFailed();
-        }
-    }
 }
diff --git a/onchain/rollups/test/foundry/consensus/authority/Authority.t.sol b/onchain/rollups/test/foundry/consensus/authority/Authority.t.sol
@@ -4,27 +4,10 @@
 /// @title Authority Test
 pragma solidity ^0.8.8;
 
-import {Test} from "forge-std/Test.sol";
 import {TestBase} from "../../util/TestBase.sol";
 import {Authority} from "contracts/consensus/authority/Authority.sol";
 import {IHistory} from "contracts/history/IHistory.sol";
 import {Vm} from "forge-std/Vm.sol";
-import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
-import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import {SimpleERC20} from "../../util/SimpleERC20.sol";
-
-contract UntransferableToken is ERC20 {
-    constructor(
-        address minter,
-        uint256 _initialSupply
-    ) ERC20("UntransferableToken", "UTFAB") {
-        _mint(minter, _initialSupply);
-    }
-
-    function transfer(address, uint256) public pure override returns (bool) {
-        return false;
-    }
-}
 
 contract HistoryReverts is IHistory {
     function submitClaim(bytes calldata) external pure override {
@@ -278,106 +261,6 @@ contract AuthorityTest is TestBase {
         assertEq(address(authority.getHistory()), address(_newHistory));
     }
 
-    function testWithdrawERC20TokensNotOwner(
-        address _owner,
-        IHistory _history,
-        address _notOwner,
-        IERC20 _token,
-        address _recipient,
-        uint256 _amount
-    ) public {
-        vm.assume(_owner != address(0));
-        vm.assume(_owner != _notOwner);
-
-        authority = new Authority(_owner);
-
-        vm.prank(_owner);
-        authority.setHistory(_history);
-
-        vm.prank(_notOwner);
-        vm.expectRevert("Ownable: caller is not the owner");
-        authority.withdrawERC20Tokens(_token, _recipient, _amount);
-    }
-
-    function testWithdrawERC20Tokens(
-        address _owner,
-        IHistory _history,
-        address _recipient,
-        uint256 _amount,
-        uint256 _balance
-    ) public {
-        vm.assume(_owner != address(0));
-        vm.assume(_recipient != address(0));
-        vm.assume(_amount <= _balance);
-        vm.assume(_balance < type(uint256).max);
-
-        authority = new Authority(_owner);
-
-        vm.prank(_owner);
-        authority.setHistory(_history);
-
-        vm.assume(_recipient != address(authority));
-
-        // mint `_balance` ERC-20 tokens for authority contract
-        IERC20 token = new SimpleERC20(address(authority), _balance);
-
-        // try to transfer more than balance
-        vm.prank(_owner);
-        vm.expectRevert("ERC20: transfer amount exceeds balance");
-        authority.withdrawERC20Tokens(token, _recipient, _balance + 1);
-
-        // since transfer fails, all balances stay the same
-        assertEq(token.balanceOf(address(authority)), _balance);
-        assertEq(token.balanceOf(_recipient), 0);
-
-        // it would succeed if the transfer amount is within balance
-        vm.prank(_owner);
-        authority.withdrawERC20Tokens(token, _recipient, _amount);
-
-        // now check balance after a successful withdraw
-        assertEq(token.balanceOf(address(authority)), _balance - _amount);
-        assertEq(token.balanceOf(_recipient), _amount);
-    }
-
-    function testWithdrawERC20TokensFailed(
-        address _owner,
-        IHistory _history,
-        address _recipient,
-        uint256 _amount,
-        uint256 _balance
-    ) public {
-        vm.assume(_owner != address(0));
-        vm.assume(_recipient != address(0));
-        vm.assume(_amount <= _balance);
-        vm.assume(_balance < type(uint256).max);
-
-        authority = new Authority(_owner);
-
-        vm.prank(_owner);
-        authority.setHistory(_history);
-
-        vm.assume(_recipient != address(authority));
-
-        // mint `_balance` ERC-20 tokens for authority contract
-        IERC20 tokenFailed = new UntransferableToken(
-            address(authority),
-            _balance
-        );
-
-        // before failed withdraw
-        assertEq(tokenFailed.balanceOf(address(authority)), _balance);
-        assertEq(tokenFailed.balanceOf(_recipient), 0);
-
-        // withdrawal fails because `transfer` returns `false`
-        vm.prank(_owner);
-        vm.expectRevert(Authority.AuthorityWithdrawalFailed.selector);
-        authority.withdrawERC20Tokens(tokenFailed, _recipient, _amount);
-
-        // after failed withdraw. All balances stay the same
-        assertEq(tokenFailed.balanceOf(address(authority)), _balance);
-        assertEq(tokenFailed.balanceOf(_recipient), 0);
-    }
-
     function testJoin(address _owner, IHistory _history, address _dapp) public {
         vm.assume(_owner != address(0));