Skip to content

Commit

Permalink
Audit fixes (#99)
Browse files Browse the repository at this point in the history
* use local signed order, rename orderinfo, fixes #2

* add version in .toml, fixes 4

* comments, fixes #6

* remove error & import, fixes #7

* use address, fixes #8

* use ++i, fixes #9

* remove UniswapX dependency

* fix eip712 hash ordering

* final audit comments
  • Loading branch information
snreynolds authored Mar 11, 2024
1 parent 53081e7 commit d32023f
Show file tree
Hide file tree
Showing 36 changed files with 221 additions and 218 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139170
139165
Original file line number Diff line number Diff line change
@@ -1 +1 @@
187003
187089
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141164
141170
Original file line number Diff line number Diff line change
@@ -1 +1 @@
239699
239780
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141170
141165
Original file line number Diff line number Diff line change
@@ -1 +1 @@
180034
180115
Original file line number Diff line number Diff line change
@@ -1 +1 @@
160343
160344
Original file line number Diff line number Diff line change
@@ -1 +1 @@
263603
263684
Original file line number Diff line number Diff line change
@@ -1 +1 @@
158669
158664
Original file line number Diff line number Diff line change
@@ -1 +1 @@
237828
237910
Original file line number Diff line number Diff line change
@@ -1 +1 @@
154373
154378
Original file line number Diff line number Diff line change
@@ -1 +1 @@
295198
295273
4 changes: 0 additions & 4 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,3 @@
[submodule "lib/permit2"]
path = lib/permit2
url = https://github.com/Uniswap/permit2
[submodule "lib/UniswapX"]
path = lib/UniswapX
url = https://github.com/Uniswap/UniswapX
branch = main
1 change: 1 addition & 0 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ no_match_path = "*/integration/*"
ffi = true
fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}, { access = "read-write", path = "out/" }, { access = "read", path = "./test/foundry-tests/interop.json"}]
optimizer_runs = 1000000
solc = "0.8.24"

[profile.integration]
no_match_path = ""
Expand Down
1 change: 0 additions & 1 deletion lib/UniswapX
Submodule UniswapX deleted from 4bc3d8
2 changes: 1 addition & 1 deletion src/base/Multicall.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ abstract contract Multicall is IMulticall {
}
results[i] = result;
unchecked {
i++;
++i;
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions src/base/ReactorErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,4 @@ interface ReactorErrors {

/// @notice Thrown when the order targets a different reactor
error InvalidReactor();

/// @notice Thrown if the order has expired
error DeadlinePassed();
}
4 changes: 2 additions & 2 deletions src/base/ReactorEvents.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ interface ReactorEvents {
/// @notice Emitted when an order is filled
/// @param orderHash The hash of the order that was filled
/// @param filler The address which executed the fill
/// @param nonce The nonce of the filled order
/// @param swapper The swapper of the filled order
event Fill(bytes32 indexed orderHash, address indexed filler, address indexed swapper, uint256 nonce);
/// @param nonce The nonce of the filled order
event Relay(bytes32 indexed orderHash, address indexed filler, address indexed swapper, uint256 nonce);
}
20 changes: 12 additions & 8 deletions src/base/ReactorStructs.sol
Original file line number Diff line number Diff line change
@@ -1,28 +1,25 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {ISignatureTransfer} from "permit2/src/interfaces/ISignatureTransfer.sol";
import {IRelayOrderReactor} from "../interfaces/IRelayOrderReactor.sol";

/// @dev Note that all of these fields are signed over. Some are hashed in the base permit and some are hashed in the passed in witness.
/// We construct the permit details and witness information.
struct RelayOrder {
// Generic order info
OrderInfo info;
RelayOrderInfo info;
// Token info for the order
Input input;
// The fee offered for the order
FeeEscalator fee;
// ecnoded data relayed to the universal router
// encoded data relayed to the universal router
bytes universalRouterCalldata;
}

/// @dev Generic order information
struct OrderInfo {
/// @dev Generic order information for a relay order
struct RelayOrderInfo {
// The address of the reactor that this order is targeting
// Note that this must be included in every order so the swapper
// signature commits to the specific reactor that they trust to fill their order properly
IRelayOrderReactor reactor;
address reactor;
// The address of the user which created the order
// Note that this must be included so that order hashes are unique by swapper
address swapper;
Expand Down Expand Up @@ -50,3 +47,10 @@ struct FeeEscalator {
// The time at which the fee becomes static
uint256 endTime;
}

/// @dev Extneral struct including a generic encoded order and swapper signature
/// The order is decoded as a RelayOrder
struct SignedOrder {
bytes order;
bytes sig;
}
2 changes: 1 addition & 1 deletion src/interfaces/IRelayOrderReactor.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {SignedOrder} from "UniswapX/src/base/ReactorStructs.sol";
import {SignedOrder} from "../base/ReactorStructs.sol";
import {IMulticall} from "./IMulticall.sol";
import {ERC20} from "solmate/src/tokens/ERC20.sol";

Expand Down
2 changes: 1 addition & 1 deletion src/lib/FeeEscalatorLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ library FeeEscalatorLib {
}

/// @notice Transforms the fee data into a SignatureTransferDetails struct needed for the permit call.
/// @dev The recipient is the fee.recipient if set, otherwise the caller provided feeRecipient
/// @dev The feeRecipient param is either the original provided feeRecipient or msg.sender
/// @param fee The order fee
/// @param feeRecipient the address to receive any specified fee
function toTransferDetails(FeeEscalator memory fee, address feeRecipient)
Expand Down
21 changes: 0 additions & 21 deletions src/lib/OrderInfoLib.sol

This file was deleted.

24 changes: 24 additions & 0 deletions src/lib/RelayOrderInfoLib.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.0;

import {RelayOrderInfo} from "../base/ReactorStructs.sol";

/// @notice Handles the EIP712 defined typehash and hashing for RelayOrderInfo
library RelayOrderInfoLib {
bytes internal constant RELAY_ORDER_INFO_TYPESTRING = abi.encodePacked(
"RelayOrderInfo(", "address reactor,", "address swapper,", "uint256 nonce,", "uint256 deadline)"
);

bytes32 internal constant RELAY_ORDER_INFO_TYPEHASH = keccak256(RELAY_ORDER_INFO_TYPESTRING);

/// @notice Hashes the orderInfo
/// @param orderInfo The info to hash
/// @return The hash of the info
function hash(RelayOrderInfo memory orderInfo) internal pure returns (bytes32) {
return keccak256(
abi.encode(
RELAY_ORDER_INFO_TYPEHASH, orderInfo.reactor, orderInfo.swapper, orderInfo.nonce, orderInfo.deadline
)
);
}
}
14 changes: 7 additions & 7 deletions src/lib/RelayOrderLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,36 @@ pragma solidity ^0.8.0;
import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
import {ISignatureTransfer} from "permit2/src/interfaces/ISignatureTransfer.sol";
import {PermitHash} from "permit2/src/libraries/PermitHash.sol";
import {RelayOrder, FeeEscalator, Input, OrderInfo} from "../base/ReactorStructs.sol";
import {RelayOrder, FeeEscalator, Input, RelayOrderInfo} from "../base/ReactorStructs.sol";
import {ReactorErrors} from "../base/ReactorErrors.sol";
import {FeeEscalatorLib} from "./FeeEscalatorLib.sol";
import {InputLib} from "./InputLib.sol";
import {OrderInfoLib} from "./OrderInfoLib.sol";
import {RelayOrderInfoLib} from "./RelayOrderInfoLib.sol";

library RelayOrderLib {
using RelayOrderLib for RelayOrder;
using FeeEscalatorLib for FeeEscalator;
using OrderInfoLib for OrderInfo;
using RelayOrderInfoLib for RelayOrderInfo;
using InputLib for Input;

// EIP712 notes that nested structs should be ordered alphabetically.
// With our added RelayOrder witness, the top level type becomes
// "PermitBatchWitnessTransferFrom(TokenPermissions[] permitted,address spender,uint256 nonce,uint256 deadline,RelayOrder witness)"
// Meaning we order the nested structs as follows:
// FeeEscalator, Input, OrderInfo, RelayOrder, TokenPermissions
// FeeEscalator, Input, RelayOrder, RelayOrderInfo TokenPermissions
string internal constant PERMIT2_ORDER_TYPE = string(
abi.encodePacked(
"RelayOrder witness)",
FeeEscalatorLib.FEE_ESCALATOR_TYPESTRING,
InputLib.INPUT_TYPESTRING,
OrderInfoLib.ORDER_INFO_TYPESTRING,
RelayOrderLib.TOPLEVEL_RELAY_ORDER_TYPESTRING,
RelayOrderInfoLib.RELAY_ORDER_INFO_TYPESTRING,
PermitHash._TOKEN_PERMISSIONS_TYPESTRING
)
);

bytes internal constant TOPLEVEL_RELAY_ORDER_TYPESTRING = abi.encodePacked(
"RelayOrder(", "OrderInfo info,", "Input input,", "FeeEscalator fee,", "bytes universalRouterCalldata)"
"RelayOrder(", "RelayOrderInfo info,", "Input input,", "FeeEscalator fee,", "bytes universalRouterCalldata)"
);

// EIP712 notes that nested structs should be ordered alphabetically:
Expand All @@ -42,7 +42,7 @@ library RelayOrderLib {
RelayOrderLib.TOPLEVEL_RELAY_ORDER_TYPESTRING,
FeeEscalatorLib.FEE_ESCALATOR_TYPESTRING,
InputLib.INPUT_TYPESTRING,
OrderInfoLib.ORDER_INFO_TYPESTRING
RelayOrderInfoLib.RELAY_ORDER_INFO_TYPESTRING
);

bytes32 internal constant FULL_RELAY_ORDER_TYPEHASH = keccak256(FULL_RELAY_ORDER_TYPESTRING);
Expand Down
9 changes: 4 additions & 5 deletions src/reactors/RelayOrderReactor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,16 @@ pragma solidity 0.8.24;

import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
import {Permit2Lib} from "permit2/src/libraries/Permit2Lib.sol";
import {ReactorEvents} from "UniswapX/src/base/ReactorEvents.sol";
import {IRelayOrderReactor} from "../interfaces/IRelayOrderReactor.sol";
import {RelayOrder} from "../base/ReactorStructs.sol";
import {RelayOrder, SignedOrder} from "../base/ReactorStructs.sol";
import {ReactorErrors} from "../base/ReactorErrors.sol";
import {Multicall} from "../base/Multicall.sol";
import {RelayOrderLib} from "../lib/RelayOrderLib.sol";
import {ERC20} from "solmate/src/tokens/ERC20.sol";
import {SignedOrder} from "UniswapX/src/base/ReactorStructs.sol";
import {ReactorEvents} from "../base/ReactorEvents.sol";

/// @notice Reactor for handling the execution of RelayOrders
/// @notice This contract MUST NOT have approvals or priviledged access
/// @notice This contract MUST NOT have approvals or privileged access
/// @notice any funds in this contract can be swept away by anyone
contract RelayOrderReactor is Multicall, ReactorEvents, ReactorErrors, IRelayOrderReactor {
using RelayOrderLib for RelayOrder;
Expand Down Expand Up @@ -46,7 +45,7 @@ contract RelayOrderReactor is Multicall, ReactorEvents, ReactorErrors, IRelayOrd
}
}

emit Fill(orderHash, msg.sender, order.info.swapper, order.info.nonce);
emit Relay(orderHash, msg.sender, order.info.swapper, order.info.nonce);
}

/// @inheritdoc IRelayOrderReactor
Expand Down
Loading

0 comments on commit d32023f

Please sign in to comment.