Skip to content

feat: recovery of excess funds #180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
michael1011 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat: recovery of excess funds #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 63 additions & 3 deletions contracts/ERC20Swap.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

pragma solidity ^0.8.33;

import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {TransferHelper} from "./TransferHelper.sol";

// @title Hash timelock contract for ERC20 tokens
contract ERC20Swap {
/// @notice The owner cannot touch funds backing active swaps. The only owner-only functions are
/// "sweepToken" (recovers ERC20 balance above the per-token accounting) and "sweepEther"
/// (recovers any ether held by the contract).
contract ERC20Swap is Ownable2Step {
// Structs

struct BatchClaimEntry {
Expand All @@ -21,7 +27,7 @@ contract ERC20Swap {
// Constants

/// @dev Version of the contract used for compatibility checks
uint8 public constant VERSION = 6;
uint8 public constant VERSION = 7;

bytes32 public constant TYPEHASH_CLAIM = keccak256(
"Claim(bytes32 preimage,uint256 amount,address tokenAddress,address refundAddress,uint256 timelock,address destination)"
Expand All @@ -37,7 +43,7 @@ contract ERC20Swap {
abi.encode(
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
keccak256("ERC20Swap"),
keccak256("6"),
keccak256("7"),
block.chainid,
address(this)
)
Expand All @@ -48,6 +54,9 @@ contract ERC20Swap {
/// @dev Mapping between value hashes of swaps and whether they have tokens locked in the contract
mapping(bytes32 => bool) public swaps;

/// @dev Total amount per ERC20 token that should be locked in the contract to cover outstanding swaps
mapping(address => uint256) public lockedAmounts;

// Errors

/// @dev Thrown when the recovered address from a signature is invalid
Expand All @@ -62,6 +71,8 @@ contract ERC20Swap {
error CommitmentCannotBeClaimedAsSwap();
/// @dev Thrown when no swap is found for the given hash
error SwapNotFound();
/// @dev Thrown when there are no excess tokens to sweep
error NoExcess();

// Events

Expand All @@ -76,6 +87,10 @@ contract ERC20Swap {

event Claim(bytes32 indexed preimageHash, bytes32 preimage);
event Refund(bytes32 indexed preimageHash);
event SweptToken(address indexed token, address indexed to, uint256 amount);
event SweptEther(address indexed to, uint256 amount);

constructor(address initialOwner) Ownable(initialOwner) {}

// External functions

Expand Down Expand Up @@ -335,6 +350,39 @@ contract ERC20Swap {
refundInternal(preimageHash, amount, tokenAddress, claimAddress, refundAddress, timelock);
}

/// Sweeps excess tokens held by the contract above the amount accounted for outstanding swaps
/// @dev ERC20 transfers cannot be refused at the contract level, so the balance may exceed the
/// sum of amounts locked by calls to "lock". The owner can recover that surplus. Tokens
/// backing active swaps remain protected by the accounting.
/// @param token Address of the token to sweep
/// @param to Recipient of the swept tokens
function sweepToken(address token, address to) external onlyOwner {
uint256 balance = IERC20(token).balanceOf(address(this));
uint256 locked = lockedAmounts[token];
require(balance > locked, NoExcess());

uint256 excess;
unchecked {
excess = balance - locked;
}

emit SweptToken(token, to, excess);
TransferHelper.safeTransferToken(token, to, excess);
}

/// Sweeps ether accidentally sent to the contract
/// @dev This contract never retains ether between transactions ("lockPrepayMinerfee" forwards
/// "msg.value" in the same call), so any ether balance is from a force-send
/// (selfdestruct / coinbase / pre-deployment transfer) and can be recovered in full.
/// @param to Recipient of the swept ether
function sweepEther(address payable to) external onlyOwner {
uint256 balance = address(this).balance;
require(balance > 0, NoExcess());

emit SweptEther(to, balance);
TransferHelper.transferEther(to, balance);
}

// Public functions

/// Locks tokens in the contract
Expand Down Expand Up @@ -367,6 +415,9 @@ contract ERC20Swap {
// Save to the state that funds were locked for this swap
swaps[hash] = true;

// Track the total amount that should be locked in the contract per token
lockedAmounts[tokenAddress] += amount;

// Emit the "Lockup" event
emit Lockup(preimageHash, amount, tokenAddress, claimAddress, refundAddress, timelock);

Expand Down Expand Up @@ -560,6 +611,9 @@ contract ERC20Swap {
// This *HAS* to be done before actually sending the tokens to avoid reentrancy
delete swaps[hash];

// Decrement the accounted locked amount for this token
lockedAmounts[tokenAddress] -= amount;

// Emit the claim event
emit Claim(preimageHash, preimage);
}
Expand Down Expand Up @@ -596,6 +650,9 @@ contract ERC20Swap {
// Reentrancy is a bigger problem when sending Ether but there is no real downside to deleting from the mapping first
delete swaps[hash];

// Decrement the accounted locked amount for this token
lockedAmounts[tokenAddress] -= amount;

// Emit the "Claim" event
emit Claim(preimageHash, preimage);
}
Expand All @@ -613,6 +670,9 @@ contract ERC20Swap {
checkSwapIsLocked(hash);
delete swaps[hash];

// Decrement the accounted locked amount for this token
lockedAmounts[tokenAddress] -= amount;

emit Refund(preimageHash);

TransferHelper.safeTransferToken(tokenAddress, refundAddress, amount);
Expand Down
2 changes: 2 additions & 0 deletions contracts/ERC20SwapTimestamp.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {ERC20Swap} from "./ERC20Swap.sol";

// @title Hash timelock contract for ERC20 tokens using block timestamps for timeouts
contract ERC20SwapTimestamp is ERC20Swap {
constructor(address initialOwner) ERC20Swap(initialOwner) {}

function currentTime() internal view override returns (uint256) {
return block.timestamp;
}
Expand Down
65 changes: 62 additions & 3 deletions contracts/EtherSwap.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

pragma solidity ^0.8.33;

import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {TransferHelper} from "./TransferHelper.sol";

// @title Hash timelock contract for Ether
contract EtherSwap {
/// @notice The owner cannot touch ether backing active swaps. The only owner-only functions are
/// "sweepEther" (recovers ether balance above the "lockedEther" accounting) and
/// "sweepToken" (recovers any ERC20 token balance held by the contract).
contract EtherSwap is Ownable2Step {
// Structs

struct BatchClaimEntry {
Expand All @@ -21,7 +27,7 @@ contract EtherSwap {
// Constants

/// @dev Version of the contract used for compatibility checks
uint8 public constant VERSION = 6;
uint8 public constant VERSION = 7;

bytes32 public constant TYPEHASH_CLAIM =
keccak256("Claim(bytes32 preimage,uint256 amount,address refundAddress,uint256 timelock,address destination)");
Expand All @@ -35,7 +41,7 @@ contract EtherSwap {
abi.encode(
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
keccak256("EtherSwap"),
keccak256("6"),
keccak256("7"),
block.chainid,
address(this)
)
Expand All @@ -46,6 +52,9 @@ contract EtherSwap {
/// @dev Mapping between value hashes of swaps and whether they have Ether locked in the contract
mapping(bytes32 => bool) public swaps;

/// @dev Total amount of Ether that should be locked in the contract to cover outstanding swaps
uint256 public lockedEther;

// Errors

/// @dev Thrown when the sent Ether amount is not greater than the prepay amount
Expand All @@ -62,6 +71,8 @@ contract EtherSwap {
error SwapAlreadyExists();
/// @dev Thrown when no swap is found for the given hash
error SwapNotFound();
/// @dev Thrown when there is nothing to sweep
error NoExcess();

// Events

Expand All @@ -75,6 +86,10 @@ contract EtherSwap {

event Claim(bytes32 indexed preimageHash, bytes32 preimage);
event Refund(bytes32 indexed preimageHash);
event SweptEther(address indexed to, uint256 amount);
event SweptToken(address indexed token, address indexed to, uint256 amount);

constructor(address initialOwner) Ownable(initialOwner) {}

// External functions

Expand Down Expand Up @@ -324,6 +339,38 @@ contract EtherSwap {
refundInternal(preimageHash, amount, claimAddress, refundAddress, timelock);
}

/// Sweeps excess Ether held by the contract above the amount accounted for outstanding swaps
/// @dev Ether can end up in the contract outside the normal lock flow via "selfdestruct" or a
/// force-send to the address before deployment. The owner can recover that surplus.
/// Ether backing active swaps remains protected by the accounting.
/// @param to Recipient of the swept Ether
function sweepEther(address payable to) external onlyOwner {
uint256 balance = address(this).balance;
uint256 locked = lockedEther;
require(balance > locked, NoExcess());

uint256 excess;
unchecked {
excess = balance - locked;
}

emit SweptEther(to, excess);
TransferHelper.transferEther(to, excess);
}

/// Sweeps ERC20 tokens accidentally sent to the contract
/// @dev EtherSwap never takes custody of ERC20 tokens as part of its normal operation, so any
/// ERC20 balance held by the contract is excess and can be recovered in full by the owner.
/// @param token Address of the token to sweep
/// @param to Recipient of the swept tokens
function sweepToken(address token, address to) external onlyOwner {
uint256 balance = IERC20(token).balanceOf(address(this));
require(balance > 0, NoExcess());

emit SweptToken(token, to, balance);
TransferHelper.safeTransferToken(token, to, balance);
}

// Public functions

/// Claims Ether locked in the contract for a specified claim address
Expand Down Expand Up @@ -485,6 +532,9 @@ contract EtherSwap {
// This *HAS* to be done before actually sending the Ether to avoid reentrancy
delete swaps[hash];

// Decrement the accounted locked Ether
lockedEther -= amount;

// Emit the claim event
emit Claim(preimageHash, preimage);
}
Expand Down Expand Up @@ -519,6 +569,9 @@ contract EtherSwap {
// This *HAS* to be done before actually sending the Ether to avoid reentrancy
delete swaps[hash];

// Decrement the accounted locked Ether
lockedEther -= amount;

// Emit the claim event
emit Claim(preimageHash, preimage);
}
Expand Down Expand Up @@ -548,6 +601,9 @@ contract EtherSwap {
// Save to the state that funds were locked for this swap
swaps[hash] = true;

// Track the total amount of Ether that should be locked in the contract
lockedEther += amount;

// Emit the "Lockup" event
emit Lockup(preimageHash, amount, claimAddress, refundAddress, timelock);
}
Expand All @@ -564,6 +620,9 @@ contract EtherSwap {
checkSwapIsLocked(hash);
delete swaps[hash];

// Decrement the accounted locked Ether
lockedEther -= amount;

emit Refund(preimageHash);

TransferHelper.transferEther(payable(refundAddress), amount);
Expand Down
2 changes: 2 additions & 0 deletions contracts/EtherSwapTimestamp.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {EtherSwap} from "./EtherSwap.sol";

// @title Hash timelock contract for Ether using block timestamps for timeouts
contract EtherSwapTimestamp is EtherSwap {
constructor(address initialOwner) EtherSwap(initialOwner) {}

function currentTime() internal view override returns (uint256) {
return block.timestamp;
}
Expand Down
7 changes: 5 additions & 2 deletions contracts/script/Deploy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ import {Router} from "../Router.sol";

contract Deploy is Script {
function run() external {
address swapOwner = vm.envAddress("SWAP_OWNER");
console.log("Using swap owner: ", swapOwner);

vm.startBroadcast();
address etherSwapAddress = address(new EtherSwap());
address etherSwapAddress = address(new EtherSwap(swapOwner));
console.log("Deployed EtherSwap: ", etherSwapAddress);
address erc20SwapAddress = address(new ERC20Swap());
address erc20SwapAddress = address(new ERC20Swap(swapOwner));
console.log("Deployed ERC20Swap: ", erc20SwapAddress);
address permit2Address = vm.envAddress("PERMIT2_ADDRESS");
console.log("Using Permit2: ", permit2Address);
Expand Down
25 changes: 24 additions & 1 deletion contracts/test/ERC20SwapFuzzTest.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {TestERC20} from "../TestERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

contract ERC20SwapFuzzTest is Test {
ERC20Swap internal swap = new ERC20Swap();
ERC20Swap internal swap = new ERC20Swap(address(this));
IERC20 internal token = new TestERC20("TestERC20", "TRC", 18, 10 ** 21);

function testLock(uint256 amount, bytes32 preimageHash, address claimAddress, uint256 timelock) external {
Expand Down Expand Up @@ -181,6 +181,29 @@ contract ERC20SwapFuzzTest is Test {
);
}

function testLockedAmountsTracksLifecycle(uint256 amount, bytes32 preimage, address claimAddress, uint256 timelock)
external
{
vm.assume(amount < token.balanceOf(address(this)));
vm.assume(amount > 0);
vm.assume(claimAddress != address(0));
vm.assume(claimAddress != address(swap));

bytes32 preimageHash = sha256(abi.encodePacked(preimage));

token.approve(address(swap), amount);
swap.lock(preimageHash, amount, address(token), claimAddress, timelock);

assertEq(swap.lockedAmounts(address(token)), amount);
assertLe(swap.lockedAmounts(address(token)), token.balanceOf(address(swap)));

vm.prank(claimAddress);
swap.claim(preimage, amount, address(token), address(this), timelock);

assertEq(swap.lockedAmounts(address(token)), 0);
assertLe(swap.lockedAmounts(address(token)), token.balanceOf(address(swap)));
}

function testHashValuesMatchesKeccak256(
bytes32 preimageHash,
uint256 amount,
Expand Down
Loading