From 0daab87a7c97c2b65b1e8039548f18bb990b0085 Mon Sep 17 00:00:00 2001 From: Jordaniza Date: Wed, 25 Sep 2024 11:47:56 +0400 Subject: [PATCH] added sweepNFT to escrow contract and more robust tests --- src/escrow/increasing/Lock.sol | 2 +- .../increasing/VotingEscrowIncreasing.sol | 15 ++ .../interfaces/IVotingEscrowIncreasing.sol | 3 + test/escrow/escrow/EscrowSweep.t.sol | 153 ++++++++++++++++++ test/escrow/escrow/Lock.t.sol | 2 + 5 files changed, 174 insertions(+), 1 deletion(-) create mode 100644 test/escrow/escrow/EscrowSweep.t.sol diff --git a/src/escrow/increasing/Lock.sol b/src/escrow/increasing/Lock.sol index 621b183..e943110 100644 --- a/src/escrow/increasing/Lock.sol +++ b/src/escrow/increasing/Lock.sol @@ -61,7 +61,7 @@ contract Lock is ILock, ERC721Enumerable, UUPSUpgradeable, DaoAuthorizable { // allow sending nfts to the escrow whitelisted[escrow] = true; - emit WhitelistSet(address(this), true); + emit WhitelistSet(address(escrow), true); } /*////////////////////////////////////////////////////////////// diff --git a/src/escrow/increasing/VotingEscrowIncreasing.sol b/src/escrow/increasing/VotingEscrowIncreasing.sol index 59369e8..ff65fa2 100644 --- a/src/escrow/increasing/VotingEscrowIncreasing.sol +++ b/src/escrow/increasing/VotingEscrowIncreasing.sol @@ -341,6 +341,21 @@ contract VotingEscrow is emit Sweep(_msgSender(), excess); } + /// @notice the sweeper can send NFTs mistakenly sent to the contract to a designated address + /// @param _tokenId the tokenId to sweep - must be currently in this contract + /// @param _to the address to send the NFT to - must be a whitelisted address for transfers + /// @dev Cannot sweep NFTs that are in the exit queue for obvious reasons + function sweepNFT(uint256 _tokenId, address _to) external nonReentrant auth(SWEEPER_ROLE) { + // if the token id is not in the contract, revert + if (IERC721EMB(lockNFT).ownerOf(_tokenId) != address(this)) revert NothingToSweep(); + + // if the token id is in the queue, we cannot sweep it + if (IExitQueue(queue).ticketHolder(_tokenId) != address(0)) revert CannotExit(); + + IERC721EMB(lockNFT).transferFrom(address(this), _to, _tokenId); + emit SweepNFT(_to, _tokenId); + } + /*/////////////////////////////////////////////////////////////// UUPS Upgrade //////////////////////////////////////////////////////////////*/ diff --git a/src/escrow/increasing/interfaces/IVotingEscrowIncreasing.sol b/src/escrow/increasing/interfaces/IVotingEscrowIncreasing.sol index d2ee260..60addc8 100644 --- a/src/escrow/increasing/interfaces/IVotingEscrowIncreasing.sol +++ b/src/escrow/increasing/interfaces/IVotingEscrowIncreasing.sol @@ -103,6 +103,7 @@ interface IWithdrawalQueue is IWithdrawalQueueErrors, IWithdrawalQueueEvents { interface ISweeperEvents { event Sweep(address indexed to, uint256 amount); + event SweepNFT(address indexed to, uint256 tokenId); } interface ISweeperErrors { @@ -112,6 +113,8 @@ interface ISweeperErrors { interface ISweeper is ISweeperEvents, ISweeperErrors { /// @notice sweeps excess tokens from the contract to a designated address function sweep() external; + + function sweepNFT(uint256 _tokenId, address _to) external; } /*/////////////////////////////////////////////////////////////// diff --git a/test/escrow/escrow/EscrowSweep.t.sol b/test/escrow/escrow/EscrowSweep.t.sol new file mode 100644 index 0000000..8f3e265 --- /dev/null +++ b/test/escrow/escrow/EscrowSweep.t.sol @@ -0,0 +1,153 @@ +pragma solidity ^0.8.17; + +import {EscrowBase} from "./EscrowBase.sol"; + +import {console2 as console} from "forge-std/console2.sol"; +import {IDAO} from "@aragon/osx/core/dao/IDAO.sol"; +import {DAO} from "@aragon/osx/core/dao/DAO.sol"; +import {Multisig, MultisigSetup} from "@aragon/multisig/MultisigSetup.sol"; + +import {ProxyLib} from "@libs/ProxyLib.sol"; + +import {IEscrowCurveUserStorage} from "@escrow-interfaces/IEscrowCurveIncreasing.sol"; +import {VotingEscrow} from "@escrow/VotingEscrowIncreasing.sol"; + +import {SimpleGaugeVoter, SimpleGaugeVoterSetup} from "src/voting/SimpleGaugeVoterSetup.sol"; +import {IGaugeVote} from "src/voting/ISimpleGaugeVoter.sol"; + +contract TestSweep is EscrowBase, IEscrowCurveUserStorage, IGaugeVote { + function setUp() public override { + super.setUp(); + + // grant the sweeper role to this contract + dao.grant({ + _who: address(this), + _where: address(escrow), + _permissionId: escrow.SWEEPER_ROLE() + }); + } + + function testCanSweepExcessTokens() public { + token.mint(address(escrow), 1000); + + assertEq(token.balanceOf(address(escrow)), 1000); + assertEq(token.balanceOf(address(this)), 0); + assertEq(escrow.totalLocked(), 0); + + vm.expectEmit(true, false, false, true); + emit Sweep(address(this), 1000); + escrow.sweep(); + + assertEq(token.balanceOf(address(escrow)), 0); + assertEq(token.balanceOf(address(this)), 1000); + } + + function testCannotSweepFromLocked() public { + address user = address(1); + token.mint(address(user), 1000); + + vm.startPrank(user); + { + token.approve(address(escrow), 1000); + escrow.createLock(1000); + } + vm.stopPrank(); + + assertEq(token.balanceOf(address(escrow)), 1000); + assertEq(token.balanceOf(address(this)), 0); + assertEq(escrow.totalLocked(), 1000); + + // first try with no excess tokens + vm.expectRevert(NothingToSweep.selector); + escrow.sweep(); + + // check that nothing changed + assertEq(token.balanceOf(address(escrow)), 1000); + assertEq(token.balanceOf(address(this)), 0); + assertEq(escrow.totalLocked(), 1000); + + // now try with excess tokens + token.mint(address(escrow), 1000); + + assertEq(token.balanceOf(address(escrow)), 2000); + + escrow.sweep(); + + assertEq(token.balanceOf(address(escrow)), 1000); + assertEq(token.balanceOf(address(this)), 1000); + assertEq(escrow.totalLocked(), 1000); + } + + function testOnlySweeperRole() public { + address notThis = address(1); + + bytes memory err = _authErr(notThis, address(escrow), escrow.SWEEPER_ROLE()); + + vm.prank(notThis); + vm.expectRevert(err); + escrow.sweep(); + + vm.prank(notThis); + vm.expectRevert(err); + escrow.sweepNFT(1, address(this)); + } + + function testCannotSweepNFTIfNotInContract() public { + // create a lock + token.mint(address(this), 1000); + token.approve(address(escrow), 1000); + uint tokenId = escrow.createLock(1000); + + // try to sweep the NFT -- should fail as it's not in the contract + vm.expectRevert(NothingToSweep.selector); + escrow.sweepNFT(tokenId, address(this)); + } + + function testCannotSweepNFTIfInQueue() public { + // create lock, enter withdrawal + token.mint(address(this), 1000); + token.approve(address(escrow), 1000); + uint tokenId = escrow.createLock(1000); + + // warp to the min lock + vm.warp(1 weeks); + + nftLock.approve(address(escrow), tokenId); + escrow.beginWithdrawal(tokenId); + + // try to sweep the NFT -- should fail as it's in the queue + vm.expectRevert(CannotExit.selector); + escrow.sweepNFT(tokenId, address(this)); + } + + function testCannotSweepNFTIfNotWhitelisted() public { + // create the lock and transfer the NFT to the contract + token.mint(address(this), 1000); + token.approve(address(escrow), 1000); + uint tokenId = escrow.createLock(1000); + nftLock.transferFrom(address(this), address(escrow), tokenId); + + // try to sweep the NFT -- should fail as this address is not whitelisted + vm.expectRevert(NotWhitelisted.selector); + escrow.sweepNFT(tokenId, address(this)); + } + + function testCanSweepNFT() public { + // create, transfer, whitelis, sweep + token.mint(address(this), 1000); + token.approve(address(escrow), 1000); + uint tokenId = escrow.createLock(1000); + nftLock.transferFrom(address(this), address(escrow), tokenId); + nftLock.setWhitelisted(address(this), true); + + assertEq(nftLock.balanceOf(address(this)), 0); + assertEq(nftLock.balanceOf(address(escrow)), 1); + + vm.expectEmit(true, false, false, true); + emit SweepNFT(address(this), tokenId); + escrow.sweepNFT(tokenId, address(this)); + + assertEq(nftLock.balanceOf(address(this)), 1); + assertEq(nftLock.balanceOf(address(escrow)), 0); + } +} diff --git a/test/escrow/escrow/Lock.t.sol b/test/escrow/escrow/Lock.t.sol index c28e355..97010f1 100644 --- a/test/escrow/escrow/Lock.t.sol +++ b/test/escrow/escrow/Lock.t.sol @@ -23,6 +23,8 @@ contract TestLockMintBurn is EscrowBase, IEscrowCurveUserStorage, IGaugeVote { address _dao, address _escrow ) public { + vm.expectEmit(true, false, false, true); + emit WhitelistSet(address(_escrow), true); Lock _nftLock = _deployLock(_escrow, _name, _symbol, _dao); assertEq(_nftLock.name(), _name);