diff --git a/onchain/rollups/.changeset/rotten-yaks-help.md b/onchain/rollups/.changeset/rotten-yaks-help.md new file mode 100644 index 00000000..1b3153c5 --- /dev/null +++ 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 new file mode 100644 index 00000000..af7d5542 --- /dev/null +++ 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/contracts@5.0.0`. +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 index 6eb19bda..fee30e77 100644 --- 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 index e15bed7b..40f603db 100644 --- 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));