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

chore: Adding Core unit tests #55

Merged
merged 13 commits into from
Feb 8, 2024
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
2 changes: 1 addition & 1 deletion .forge-snapshots/settler_metaTxn_uniswapV3_USDT-WETH.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
160735
160734
Original file line number Diff line number Diff line change
@@ -1 +1 @@
138683
138686
Original file line number Diff line number Diff line change
@@ -1 +1 @@
181465
181468
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ Note: The following is more akin to `gasLimit` than it is `gasUsed`, this is due
| VIP | DEX | Pair | Gas | % |
| ------------------- | ---------- | --------- | ------ | ------ |
| 0x V4 VIP | Uniswap V3 | USDC/WETH | 125117 | 0.00% |
| 0x V4 Multiplex | Uniswap V3 | USDC/WETH | 138683 | 10.84% |
| 0x V4 Multiplex | Uniswap V3 | USDC/WETH | 138686 | 10.85% |
| Settler VIP (warm) | Uniswap V3 | USDC/WETH | 134857 | 7.78% |
| AllowanceHolder VIP | Uniswap V3 | USDC/WETH | 130694 | 4.46% |
| UniswapRouter V3 | Uniswap V3 | USDC/WETH | 121137 | -3.18% |
Expand Down Expand Up @@ -73,7 +73,7 @@ Note: The following is more akin to `gasLimit` than it is `gasUsed`, this is due
| Settler | Uniswap V3 | DAI/WETH | 154057 | -36.05% |
| | | | | |
| 0x V4 Multiplex | Uniswap V3 | USDT/WETH | 243700 | 0.00% |
| Settler | Uniswap V3 | USDT/WETH | 160735 | -34.04% |
| Settler | Uniswap V3 | USDT/WETH | 160734 | -34.04% |
| | | | | |

| OTC | DEX | Pair | Gas | % |
Expand Down
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ no_match_path = "*/integration/*"
fuzz_runs = 10000
# needed for marktoda/forge-gas-snapshot
ffi = true
fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}]
fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"},{ access = "read", path = "out"}]
evm_version = "shanghai"

[profile.integration]
Expand Down
12 changes: 0 additions & 12 deletions script/Counter.s.sol

This file was deleted.

6 changes: 4 additions & 2 deletions src/core/Basic.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ abstract contract Basic is Permit2PaymentAbstract {
revert ConfusedDeputy();
}

bool success;
bytes memory returnData;
uint256 value;
if (sellToken == IERC20(ETH_ADDRESS)) {
value = address(this).balance.mulDiv(bips, 10_000);
if (data.length == 0) {
if (offset != 0) revert InvalidOffset();
(bool success, bytes memory returnData) = payable(pool).call{value: value}("");
(success, returnData) = payable(pool).call{value: value}("");
success.maybeRevert(returnData);
return;
} else {
Expand All @@ -56,7 +58,7 @@ abstract contract Basic is Permit2PaymentAbstract {
sellToken.safeApproveIfBelow(pool, amount);
}
}
(bool success, bytes memory returnData) = payable(pool).call{value: value}(data);
(success, returnData) = payable(pool).call{value: value}(data);
success.maybeRevert(returnData);
// forbid sending data to EOAs
if (returnData.length == 0 && pool.code.length == 0) revert InvalidTarget();
Expand Down
21 changes: 11 additions & 10 deletions src/core/OtcOrderSettlement.sol
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
bool partialFillAllowed;
}

/// @dev Emitted whenever an OTC order is filled.
/// @dev Emitted whenever an Otc order is filled.
/// @param orderHash The canonical hash of the order. Formed as an EIP712 struct hash. See below.
/// @param maker The maker of the order.
/// @param taker The taker of the order.
/// @param makerTokenFilledAmount How much maker token was filled.
/// @param takerTokenFilledAmount How much taker token was filled.
/// @param makerTokenFilledAmount Amount of maker token filled.
/// @param takerTokenFilledAmount Amount of taker token filled.
event OtcOrderFilled(
bytes32 indexed orderHash,
address maker,
Expand Down Expand Up @@ -79,9 +79,8 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
}

/// @dev Settle an OtcOrder between maker and taker transfering funds directly between
/// the counterparties. Two Permit2 signatures are consumed, with the maker Permit2 containing
/// a witness of the OtcOrder.
/// This variant also includes a fee where the taker or maker pays the fee recipient
/// the counterparties. Either two Permit2 signatures are consumed, with the maker Permit2 containing
/// a witness of the OtcOrder, or AllowanceHolder is supported for the taker payment.
function fillOtcOrder(
address recipient,
ISignatureTransfer.PermitTransferFrom memory makerPermit,
Expand All @@ -101,12 +100,12 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
bytes32 witness = _hashConsideration(consideration);
// There is no taker witness (see below)

// Maker pays recipient (optional fee)
// Maker pays recipient
_transferFrom(makerPermit, makerTransferDetails, maker, witness, CONSIDERATION_WITNESS, makerSig, false);
// Taker pays Maker (optional fee)
// Taker pays Maker
// We don't need to include a witness here. Taker is `_msgSender()`, so
// `recipient` and the maker's details are already authenticated. We're just
// using PERMIT2 to move tokens, not to provide authentication.
// using Permit2 or AllowanceHolder to move tokens, not to provide authentication.
_transferFrom(takerPermit, takerTransferDetails, _msgSender(), takerSig);

emit OtcOrderFilled(
Expand All @@ -128,6 +127,7 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
/// @dev Settle an OtcOrder between maker and taker transfering funds directly between
/// the counterparties. Both Maker and Taker have signed the same order, and submission
/// is via a third party
/// @dev `takerWitness` is not calculated nor verified here as caller is trusted
function fillOtcOrderMetaTxn(
address recipient,
ISignatureTransfer.PermitTransferFrom memory makerPermit,
Expand All @@ -151,6 +151,7 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
makerConsideration.counterparty = taker;

bytes32 makerWitness = _hashConsideration(makerConsideration);
// Note: takerWitness is not calculated here, but in the caller code

_transferFrom(makerPermit, makerTransferDetails, maker, makerWitness, CONSIDERATION_WITNESS, makerSig, false);
_transferFrom(takerPermit, takerTransferDetails, taker, takerWitness, ACTIONS_AND_SLIPPAGE_WITNESS, takerSig);
Expand All @@ -169,7 +170,7 @@ abstract contract OtcOrderSettlement is SettlerAbstract {
/// @dev Settle an OtcOrder between maker and Settler retaining funds in this contract.
/// @dev pre-condition: msgSender has been authenticated against the requestor
/// One Permit2 signature is consumed, with the maker Permit2 containing a witness of the OtcOrder.
// In this variant, Maker pays Settler and Settler pays Maker
// In this variant, Maker pays recipient and Settler pays Maker
function fillOtcOrderSelfFunded(
address recipient,
ISignatureTransfer.PermitTransferFrom memory permit,
Expand Down
Loading
Loading