Skip to content
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.

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

C4 31,46 #39

Merged
merged 19 commits into from
Jul 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
54 changes: 17 additions & 37 deletions packages/contracts/contracts/TransactionManager.sol
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.4;

import "./interfaces/IFulfillHelper.sol";
import "./interfaces/IFulfillInterpreter.sol";
import "./interfaces/ITransactionManager.sol";
import "./lib/LibAsset.sol";
import "./interpreters/FulfillInterpreter.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";

Expand Down Expand Up @@ -73,8 +74,11 @@ contract TransactionManager is ReentrancyGuard, ITransactionManager {
/// @dev Maximum timeout
uint256 public constant MAX_TIMEOUT = 30 days; // 720 hours

constructor(uint256 _chainId) {
IFulfillInterpreter private interpreter;

constructor(uint256 _chainId, address _interpreter) {
chainId = _chainId;
interpreter = FulfillInterpreter(_interpreter);
}

/// @notice This is used by any router to increase their available
Expand Down Expand Up @@ -373,45 +377,21 @@ contract TransactionManager is ReentrancyGuard, ITransactionManager {
// address in case the call fails so the funds dont remain
// locked.

// First, approve the funds to the helper if needed
// First, transfer the funds to the helper if needed
if (!LibAsset.isEther(txData.receivingAssetId) && toSend > 0) {
LibAsset.approveERC20(txData.receivingAssetId, txData.callTo, toSend);
LibAsset.transferERC20(txData.receivingAssetId, address(interpreter), toSend);
}

// Next, call `addFunds` on the helper. Helpers should internally
// Next, call `execute` on the helper. Helpers should internally
// track funds to make sure no one user is able to take all funds
// for tx
if (toSend > 0) {
try
IFulfillHelper(txData.callTo).addFunds{ value: LibAsset.isEther(txData.receivingAssetId) ? toSend : 0}(
txData.user,
txData.transactionId,
txData.receivingAssetId,
toSend
)
{} catch {
// Regardless of error within the callData execution, send funds
// to the predetermined fallback address
LibAsset.transferAsset(txData.receivingAssetId, payable(txData.receivingAddress), toSend);
}
}

// Call `execute` on the helper
try
IFulfillHelper(txData.callTo).execute(
txData.user,
txData.transactionId,
txData.receivingAssetId,
toSend,
callData
)
{} catch {
// Regardless of error within the callData execution, send funds
// to the predetermined fallback address
if (toSend > 0) {
LibAsset.transferAsset(txData.receivingAssetId, payable(txData.receivingAddress), toSend);
}
}
// for tx, and handle the case of reversions
interpreter.execute{ value: LibAsset.isEther(txData.receivingAssetId) ? toSend : 0}(
payable(txData.callTo),
txData.receivingAssetId,
payable(txData.receivingAddress),
toSend,
callData
);
}
}

Expand Down
19 changes: 0 additions & 19 deletions packages/contracts/contracts/interfaces/IFulfillHelper.sol

This file was deleted.

13 changes: 13 additions & 0 deletions packages/contracts/contracts/interfaces/IFulfillInterpreter.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.4;

interface IFulfillInterpreter {
// TODO: include transaction id?
function execute(
address payable callTo,
address assetId,
address payable fallbackAddress,
uint256 amount,
bytes calldata callData
) external payable;
}
35 changes: 35 additions & 0 deletions packages/contracts/contracts/interpreters/FulfillInterpreter.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.4;

import "../interfaces/IFulfillInterpreter.sol";
import "../lib/LibAsset.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

contract FulfillInterpreter is ReentrancyGuard, IFulfillInterpreter {
function execute(
address payable callTo,
address assetId,
address payable fallbackAddress,
uint256 amount,
bytes calldata callData
) override external payable nonReentrant {
// If it is not ether, approve the callTo
bool isEther = LibAsset.isEther(assetId);
if (!isEther) {
LibAsset.increaseERC20Allowance(assetId, callTo, amount);
}

// Try to execute the callData
// the low level call will return `false` if its execution reverts
(bool success, bytes memory returnData) = payable(callTo).call{value: isEther ? amount : 0}(callData);

if (!success) {
// If it fails, transfer to fallback
LibAsset.transferAsset(assetId, fallbackAddress, amount);
// Decrease allowance
if (!isEther) {
LibAsset.decreaseERC20Allowance(assetId, callTo, amount);
}
}
}
}
12 changes: 10 additions & 2 deletions packages/contracts/contracts/lib/LibAsset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,20 @@ library LibAsset {
IERC20(assetId).transferFrom(from, to, amount);
}

function approveERC20(
function increaseERC20Allowance(
address assetId,
address spender,
uint256 amount
) internal {
IERC20(assetId).approve(spender, amount);
SafeERC20.safeIncreaseAllowance(IERC20(assetId), spender, amount);
}

function decreaseERC20Allowance(
address assetId,
address spender,
uint256 amount
) internal {
SafeERC20.safeDecreaseAllowance(IERC20(assetId), spender, amount);
}

// This function is a wrapper for transfers of Ether or ERC20 tokens,
Expand Down
34 changes: 34 additions & 0 deletions packages/contracts/contracts/test/Counter.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.4;

import "../lib/LibAsset.sol";

contract Counter {
bool public shouldRevert;
uint256 public count = 0;

constructor() {
shouldRevert = false;
}

function setShouldRevert(bool value) public {
shouldRevert = value;
}

function increment() public {
require(!shouldRevert, "increment: shouldRevert is true");
count += 1;
}

function incrementAndSend(address assetId, address recipient, uint256 amount) public payable {
if (LibAsset.isEther(assetId)) {
require(msg.value == amount, "incrementAndSend: INVALID_ETH_AMOUNT");
} else {
require(msg.value == 0, "incrementAndSend: ETH_WITH_ERC");
LibAsset.transferFromERC20(assetId, msg.sender, address(this), amount);
}
increment();

LibAsset.transferAsset(assetId, payable(recipient), amount);
}
}
69 changes: 0 additions & 69 deletions packages/contracts/contracts/test/TestFulfillHelper.sol

This file was deleted.

8 changes: 7 additions & 1 deletion packages/contracts/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,15 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment): Promise<voi
}
console.log("deployer: ", deployer);

const interpreter = await hre.deployments.deploy("FulfillInterpreter", {
from: deployer,
args: [],
log: true,
});

await hre.deployments.deploy("TransactionManager", {
from: deployer,
args: [chainId],
args: [chainId, interpreter.address],
log: true,
});

Expand Down
Loading